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

Remove unnecessary clone() functions in hull #1089

Merged
merged 3 commits into from
Jan 4, 2024
Merged

Remove unnecessary clone() functions in hull #1089

merged 3 commits into from
Jan 4, 2024

Conversation

jklott
Copy link
Contributor

@jklott jklott commented Jan 4, 2024

There is a TODO specifically pointing out some unneeded clone() functions being used which this PR addresses. A total of 5 clones can be removed safely.

@tsoutsman tsoutsman self-requested a review January 4, 2024 12:19
Copy link
Member

@tsoutsman tsoutsman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

Either between when that code was written and now the compiler gained the ability to reason about return statements in loop bodies, or I underestimated the compiler when writing the code. Judging by the last clearly unnecessary clone, it was probably the latter.

@tsoutsman tsoutsman merged commit ffb5e8b into theseus-os:theseus_main Jan 4, 2024
3 checks passed
github-actions bot pushed a commit that referenced this pull request Jan 4, 2024
* Commands can take ownership of `previous_output` as they then overwrite it anyway.

* The final command can take ownership of all streams.

---------

Co-authored-by: Poxxy <41162160+Poxxy@users.noreply.github.com>
Co-authored-by: Klim Tsoutsman <klim@tsoutsman.com> ffb5e8b
github-actions bot pushed a commit to tsoutsman/Theseus that referenced this pull request Jan 6, 2024
…1089)

* Commands can take ownership of `previous_output` as they then overwrite it anyway.

* The final command can take ownership of all streams.

---------

Co-authored-by: Poxxy <41162160+Poxxy@users.noreply.github.com>
Co-authored-by: Klim Tsoutsman <klim@tsoutsman.com> ffb5e8b
github-actions bot pushed a commit to sunshine-lcc/Theseus that referenced this pull request Feb 7, 2024
…1089)

* Commands can take ownership of `previous_output` as they then overwrite it anyway.

* The final command can take ownership of all streams.

---------

Co-authored-by: Poxxy <41162160+Poxxy@users.noreply.github.com>
Co-authored-by: Klim Tsoutsman <klim@tsoutsman.com> ffb5e8b
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants