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 MSVC compilation error on EVMVersion comparison #15675

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from

Conversation

gmh5225
Copy link

@gmh5225 gmh5225 commented Dec 24, 2024

No description provided.

Copy link

Thank you for your contribution to the Solidity compiler! A team member will follow up shortly.

If you haven't read our contributing guidelines and our review checklist before, please do it now, this makes the reviewing process and accepting your contribution smoother.

If you have any questions or need our help, feel free to post them in the PR or talk to us directly on the #solidity-dev channel on Matrix.

@r0qs
Copy link
Member

r0qs commented Dec 26, 2024

Hi @gmh5225, thank you for your contribution. Could you please clarify what kind of compilation error you encountered on MSVC? The changes in the PR seem unnecessary to me.

The EVMVersion class implements comparability by inheriting from boost::less_than_comparable and boost::equality_comparable. These templates provide comparison operators such as >, <=, and >=, derived from the definitions of operator< and operator==. See:

class EVMVersion:
boost::less_than_comparable<EVMVersion>,
boost::equality_comparable<EVMVersion>

bool operator==(EVMVersion const& _other) const { return m_version == _other.m_version; }
bool operator<(EVMVersion const& _other) const { return m_version < _other.m_version; }

Maybe it is a Boost-related issue? Could you please provide some description of the problem you are facing so we can better understand and address it?

@gmh5225
Copy link
Author

gmh5225 commented Dec 26, 2024

image

The error is: solidity\liblangutil\EVMVersion.cpp(32,2): error C2676: binary '>=': 'const solidity::langutil::EVMVersion::Version' does not define this operator or a conversion to a type acceptable to the predefined operator

The issue is not Boost-related. While the class has overloaded operator== and operator<, it doesn't have an overloaded operator>=.

The issue can be reproduced in Debug mode with MSVC.

@r0qs

@cameel cameel changed the title fix compilation error on MSVC Fix MSVC 0compilation error on EVMVersion comparison Dec 27, 2024
@cameel cameel added the EOF label Dec 27, 2024
@cameel
Copy link
Member

cameel commented Dec 27, 2024

This is comparing the object with a number. We don't have an operator defined for that so it must be working only due to some implicit conversion. Not sure why only MSVC would be choking on that, probably some subtle implementation difference between compilers, but a cleaner way to fix it would be something like: *this >= prague().

liblangutil/EVMVersion.cpp Outdated Show resolved Hide resolved
@cameel cameel changed the title Fix MSVC 0compilation error on EVMVersion comparison Fix MSVC compilation error on EVMVersion comparison Dec 27, 2024
@gmh5225 gmh5225 requested a review from cameel December 28, 2024 17:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants