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

Fix EnumeratorHandle compilation #5109

Merged
merged 1 commit into from
Jan 23, 2025

Conversation

qobilidop
Copy link
Member

@qobilidop qobilidop commented Jan 23, 2025

Fixes #5107.

@qobilidop qobilidop requested review from asl and fruffy January 23, 2025 04:57
@qobilidop qobilidop force-pushed the fix-enumerator-compilation branch from 0349a7f to 010c503 Compare January 23, 2025 05:01
@@ -630,6 +631,11 @@ const EnumeratorHandle<T> &EnumeratorHandle<T>::operator++() {
return *this;
}

template <typename T>
bool EnumeratorHandle<T>::operator==(const EnumeratorHandle<T> &other) const {
return !(*this != other);
Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry for the noise comment, but is this way of defining operator== common in C++?

Copy link
Contributor

Choose a reason for hiding this comment

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

Usually the opposite (operator!= is implemented from operator==), however, here we already having operator!= with quite peculiar semantics.

Copy link
Contributor

Choose a reason for hiding this comment

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

Usually the opposite (operator!= is implemented from operator==),

Indeed, with C++20, you don't even need to do that -- if you define operator==, the compiler will synthesize operator!= for you

@@ -630,6 +631,11 @@ const EnumeratorHandle<T> &EnumeratorHandle<T>::operator++() {
return *this;
}

template <typename T>
bool EnumeratorHandle<T>::operator==(const EnumeratorHandle<T> &other) const {
return !(*this != other);
Copy link
Contributor

Choose a reason for hiding this comment

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

I guess this is as good as can be done, but the (existing) EnumeratorHandle::operator!= right after this looks suspect to me.

@@ -630,6 +631,11 @@ const EnumeratorHandle<T> &EnumeratorHandle<T>::operator++() {
return *this;
}

template <typename T>
bool EnumeratorHandle<T>::operator==(const EnumeratorHandle<T> &other) const {
return !(*this != other);
Copy link
Contributor

Choose a reason for hiding this comment

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

Usually the opposite (operator!= is implemented from operator==),

Indeed, with C++20, you don't even need to do that -- if you define operator==, the compiler will synthesize operator!= for you

@fruffy fruffy added the core Topics concerning the core segments of the compiler (frontend, midend, parser) label Jan 23, 2025
@qobilidop qobilidop added this pull request to the merge queue Jan 23, 2025
Merged via the queue into p4lang:main with commit 660779e Jan 23, 2025
20 checks passed
@qobilidop qobilidop deleted the fix-enumerator-compilation branch January 23, 2025 20:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Topics concerning the core segments of the compiler (frontend, midend, parser)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add EnumeratorHandle::operator== to avoid compilation error with Google internal toolchain
5 participants