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

Do not toggle BOOST_NO_ARGUMENT_DEPENDENT_LOOKUP in CMakelists.txt #4181

Merged
merged 3 commits into from
Dec 15, 2023

Conversation

fruffy
Copy link
Collaborator

@fruffy fruffy commented Oct 1, 2023

Fixes #4147.

@fruffy fruffy requested a review from ChrisDodd October 2, 2023 13:38
@jafingerhut
Copy link
Contributor

I could approve this, but would prefer if someone closer to the C++ implementation could do so. @ChrisDodd would you have any time to take a look? The changes are short, but not sure if thinking about the possible consequences of the change are.

@fruffy fruffy added the core Topics concerning the core segments of the compiler (frontend, midend, parser) label Nov 3, 2023
@fruffy
Copy link
Collaborator Author

fruffy commented Dec 12, 2023

I could approve this, but would prefer if someone closer to the C++ implementation could do so. @ChrisDodd would you have any time to take a look? The changes are short, but not sure if thinking about the possible consequences of the change are.

I do not know who is qualified to review these types of PR at this point.

@jafingerhut
Copy link
Contributor

@vlstill Do you have some expertise in the files affected here such that you might be able to give this a thoughtful review?

@vlstill vlstill self-requested a review December 14, 2023 08:10
Copy link
Contributor

@vlstill vlstill left a comment

Choose a reason for hiding this comment

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

Seems like these were all workarounds for some pretty old GCC bugs. Since the comment mentions GCC 4.8 and since this compiles and passes test with GCC 9.4 (which is almost the oldest supported) I guess it is safe to delete the workarounds now. As for the boost macro, that should be safe too, there should be no real conflict between the get versions.

It would be nice to have at least basic unit test for get though.

@fruffy fruffy merged commit 99564ff into main Dec 15, 2023
13 checks passed
@fruffy fruffy deleted the fruffy/arg_dependent_lookup branch December 15, 2023 09:19
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.

Build errors with Boost 1.83
3 participants