Fix C++ command group recursive constructor bug (#1941)

Passing command groups as lvalue-references to other command groups should be illegal, as their copy constructors have been deleted. However, copy constructors are const-qualified. This led to a very obscure bug where passing a command group by lvalue to another command group would result in a valid template expansion 'looking like' a copy constructor, and being preferred to the deleted copy constructor. This would result in constructor recursion (the expanded constructor would, in an attempt to call the copy constructor, call itself), and an eventual segfault when the stack inevitably overflowed.

This fixes the problem by explicitly deleting the problematic constructor signature - attempting to do this now (correctly) generates a compilation error.

wpilibc/src/main/native/include/frc2/command/ParallelCommandGroup.h

@@ -7,6 +7,11 @@
 
 #pragma once
 
+#ifdef _WIN32
+#pragma warning(push)
+#pragma warning(disable : 4521)
+#endif
+
 #include <memory>
 #include <unordered_map>
 #include <utility>
@@ -57,7 +62,12 @@ class ParallelCommandGroup
   // No copy constructors for commandgroups
   ParallelCommandGroup(const ParallelCommandGroup&) = delete;
 
-  template <class... Types>
+  // Prevent template expansion from emulating copy ctor
+  ParallelCommandGroup(ParallelCommandGroup&) = delete;
+
+  template <class... Types,
+            typename = std::enable_if_t<std::conjunction_v<
+                std::is_base_of<Command, std::remove_reference_t<Types>>...>>>
   void AddCommands(Types&&... commands) {
     std::vector<std::unique_ptr<Command>> foo;
     ((void)foo.emplace_back(std::make_unique<std::remove_reference_t<Types>>(
@@ -84,3 +94,7 @@ class ParallelCommandGroup
   bool isRunning = false;
 };
 }  // namespace frc2
+
+#ifdef _WIN32
+#pragma warning(pop)
+#endif

wpilibc/src/main/native/include/frc2/command/ParallelDeadlineGroup.h

@@ -7,6 +7,11 @@
 
 #pragma once
 
+#ifdef _WIN32
+#pragma warning(push)
+#pragma warning(disable : 4521)
+#endif
+
 #include <memory>
 #include <unordered_map>
 #include <utility>
@@ -65,6 +70,9 @@ class ParallelDeadlineGroup
   // No copy constructors for command groups
   ParallelDeadlineGroup(const ParallelDeadlineGroup&) = delete;
 
+  // Prevent template expansion from emulating copy ctor
+  ParallelDeadlineGroup(ParallelDeadlineGroup&) = delete;
+
   template <class... Types,
             typename = std::enable_if_t<std::conjunction_v<
                 std::is_base_of<Command, std::remove_reference_t<Types>>...>>>
@@ -97,3 +105,7 @@ class ParallelDeadlineGroup
   bool isRunning = false;
 };
 }  // namespace frc2
+
+#ifdef _WIN32
+#pragma warning(pop)
+#endif

wpilibc/src/main/native/include/frc2/command/ParallelRaceGroup.h

@@ -7,6 +7,11 @@
 
 #pragma once
 
+#ifdef _WIN32
+#pragma warning(push)
+#pragma warning(disable : 4521)
+#endif
+
 #include <memory>
 #include <set>
 #include <unordered_map>
@@ -48,6 +53,9 @@ class ParallelRaceGroup
   // No copy constructors for command groups
   ParallelRaceGroup(const ParallelRaceGroup&) = delete;
 
+  // Prevent template expansion from emulating copy ctor
+  ParallelRaceGroup(ParallelRaceGroup&) = delete;
+
   template <class... Types>
   void AddCommands(Types&&... commands) {
     std::vector<std::unique_ptr<Command>> foo;
@@ -76,3 +84,7 @@ class ParallelRaceGroup
   bool isRunning = false;
 };
 }  // namespace frc2
+
+#ifdef _WIN32
+#pragma warning(pop)
+#endif

wpilibc/src/main/native/include/frc2/command/PerpetualCommand.h

@@ -7,6 +7,11 @@
 
 #pragma once
 
+#ifdef _WIN32
+#pragma warning(push)
+#pragma warning(disable : 4521)
+#endif
+
 #include <memory>
 #include <utility>
 
@@ -54,6 +59,9 @@ class PerpetualCommand : public CommandHelper<CommandBase, PerpetualCommand> {
   // No copy constructors for command groups
   PerpetualCommand(const PerpetualCommand& other) = delete;
 
+  // Prevent template expansion from emulating copy ctor
+  PerpetualCommand(PerpetualCommand&) = delete;
+
   void Initialize() override;
 
   void Execute() override;
@@ -64,3 +72,7 @@ class PerpetualCommand : public CommandHelper<CommandBase, PerpetualCommand> {
   std::unique_ptr<Command> m_command;
 };
 }  // namespace frc2
+
+#ifdef _WIN32
+#pragma warning(pop)
+#endif

wpilibc/src/main/native/include/frc2/command/SelectCommand.h

@@ -7,6 +7,11 @@
 
 #pragma once
 
+#ifdef _WIN32
+#pragma warning(push)
+#pragma warning(disable : 4521)
+#endif
+
 #include <memory>
 #include <unordered_map>
 #include <utility>
@@ -88,6 +93,9 @@ class SelectCommand : public CommandHelper<CommandBase, SelectCommand<Key>> {
   // No copy constructors for command groups
   SelectCommand(const SelectCommand& other) = delete;
 
+  // Prevent template expansion from emulating copy ctor
+  SelectCommand(SelectCommand&) = delete;
+
   /**
    * Creates a new selectcommand.
    *
@@ -139,3 +147,7 @@ void SelectCommand<T>::Initialize() {
 }
 
 }  // namespace frc2
+
+#ifdef _WIN32
+#pragma warning(pop)
+#endif

wpilibc/src/main/native/include/frc2/command/SequentialCommandGroup.h

@@ -7,6 +7,11 @@
 
 #pragma once
 
+#ifdef _WIN32
+#pragma warning(push)
+#pragma warning(disable : 4521)
+#endif
+
 #include <limits>
 #include <memory>
 #include <utility>
@@ -61,6 +66,9 @@ class SequentialCommandGroup
   // No copy constructors for command groups
   SequentialCommandGroup(const SequentialCommandGroup&) = delete;
 
+  // Prevent template expansion from emulating copy ctor
+  SequentialCommandGroup(SequentialCommandGroup&) = delete;
+
   template <class... Types,
             typename = std::enable_if_t<std::conjunction_v<
                 std::is_base_of<Command, std::remove_reference_t<Types>>...>>>
@@ -90,3 +98,7 @@ class SequentialCommandGroup
   bool m_runWhenDisabled{true};
 };
 }  // namespace frc2
+
+#ifdef _WIN32
+#pragma warning(pop)
+#endif