Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[cpp cmd] BREAKING: Delete UB-causing rvalue variants of CommandPtr methods #4923

Merged
merged 4 commits into from
Jan 12, 2023
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
Expand All @@ -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();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Starlight220 marked this conversation as resolved.
Show resolved Hide resolved
"individually!");
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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
Expand All @@ -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"
Expand All @@ -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.
Expand Down