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

Generalize LLPointer's comparison operators to allow comparable LLPointer types. #2681

Merged
merged 4 commits into from
Sep 25, 2024

Conversation

nat-goodspeed
Copy link
Collaborator

Also remove hugely redundant restatement of the LLPointer class for LLConstPointer, which can be effected with an alias template.

Use canonical copy-and-swap idiom for LLPointer's assignment operators.

That is, both `LLPointer::operator==()` and `operator!=()` and the free function
`operator==()` and `operator!=()` now accept pointer types other than the type
of the subject `LLPointer`, letting the compiler apply implicit pointer
conversions or diagnose an error.
Instead of restating the whole class, requiring changes to be made in parallel
(which has already failed), just make a template alias.
This affords strong exception safety. Also, eliminating the conditional may
improve speed.
Given templated constructors and assignment operators, it's tempting to remove
specific constructors and assignment operators with the same LLPointer<Type>
parameters. That turns out to be a mistake. Add comments to warn future
maintainers.
Copy link
Contributor

@marchcat marchcat left a comment

Choose a reason for hiding this comment

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

Awesome, thank you!

@marchcat marchcat merged commit 4b2b94f into marchcat/xcode-16 Sep 25, 2024
11 checks passed
@marchcat marchcat deleted the nat/xcode-16 branch September 25, 2024 15:52
@github-actions github-actions bot locked and limited conversation to collaborators Sep 25, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants