[commands] Delete UB-causing rvalue variants of CommandPtr methods (#4923)

Co-authored-by: Ryan Blue <[email protected]>

Author: Starlight220

cpp/frc2/command/CommandPtr.cpp

@@ -226,7 +226,7 @@ CommandPtr CommandPtr::WithName(std::string_view name) && {
   return std::move(wrapper).ToPtr();
 }
 
-CommandBase* CommandPtr::get() const {
+CommandBase* CommandPtr::get() const& {
   AssertValid();
   return m_ptr.get();
 }
@@ -236,27 +236,27 @@ std::unique_ptr<CommandBase> CommandPtr::Unwrap() && {
   return std::move(m_ptr);
 }
 
-void CommandPtr::Schedule() const {
+void CommandPtr::Schedule() const& {
   AssertValid();
   CommandScheduler::GetInstance().Schedule(*this);
 }
 
-void CommandPtr::Cancel() const {
+void CommandPtr::Cancel() const& {
   AssertValid();
   CommandScheduler::GetInstance().Cancel(*this);
 }
 
-bool CommandPtr::IsScheduled() const {
+bool CommandPtr::IsScheduled() const& {
   AssertValid();
   return CommandScheduler::GetInstance().IsScheduled(*this);
 }
 
-bool CommandPtr::HasRequirement(Subsystem* requirement) const {
+bool CommandPtr::HasRequirement(Subsystem* requirement) const& {
   AssertValid();
   return m_ptr->HasRequirement(requirement);
 }
 
-CommandPtr::operator bool() const {
+CommandPtr::operator bool() const& {
   return m_ptr.operator bool();
 }
 

cpp/frc2/command/CommandScheduler.cpp

@@ -433,9 +433,10 @@ void CommandScheduler::OnCommandFinish(Action action) {
 
 void CommandScheduler::RequireUngrouped(const Command* command) {
   if (command->IsComposed()) {
-    throw FRC_MakeError(
-        frc::err::CommandIllegalUse,
-        "Commands cannot be added to more than one CommandGroup");
+    throw FRC_MakeError(frc::err::CommandIllegalUse,
+                        "Commands that have been composed may not be added to "
+                        "another composition or scheduled "
+                        "individually!");
   }
 }
 

include/frc2/command/CommandPtr.h

@@ -232,7 +232,10 @@ class CommandPtr final {
   /**
    * Get a raw pointer to the held command.
    */
-  CommandBase* get() const;
+  CommandBase* get() const&;
+
+  // Prevent calls on a temporary, as the returned pointer would be invalid
+  CommandBase* get() && = delete;
 
   /**
    * Convert to the underlying unique_ptr.
@@ -242,13 +245,19 @@ class CommandPtr final {
   /**
    * Schedules this command.
    */
-  void Schedule() const;
+  void Schedule() const&;
+
+  // Prevent calls on a temporary, as the returned pointer would be invalid
+  void Schedule() && = delete;
 
   /**
    * Cancels this command. Will call End(true). Commands will be canceled
    * regardless of interruption behavior.
    */
-  void Cancel() const;
+  void Cancel() const&;
+
+  // Prevent calls on a temporary, as the returned pointer would be invalid
+  void Cancel() && = delete;
 
   /**
    * Whether or not the command is currently scheduled. Note that this does not
@@ -257,7 +266,10 @@ class CommandPtr final {
    *
    * @return Whether the command is scheduled.
    */
-  bool IsScheduled() const;
+  bool IsScheduled() const&;
+
+  // Prevent calls on a temporary, as the returned pointer would be invalid
+  void IsScheduled() && = delete;
 
   /**
    * Whether the command requires a given subsystem.  Named "HasRequirement"
@@ -267,12 +279,18 @@ class CommandPtr final {
    * @param requirement the subsystem to inquire about
    * @return whether the subsystem is required
    */
-  bool HasRequirement(Subsystem* requirement) const;
+  bool HasRequirement(Subsystem* requirement) const&;
+
+  // Prevent calls on a temporary, as the returned pointer would be invalid
+  void HasRequirement(Subsystem* requirement) && = delete;
 
   /**
    * Check if this CommandPtr object is valid and wasn't moved-from.
    */
-  explicit operator bool() const;
+  explicit operator bool() const&;
+
+  // Prevent calls on a temporary, as the returned pointer would be invalid
+  explicit operator bool() && = delete;
 
   /**
    * Convert a vector of CommandPtr objects to their underlying unique_ptrs.