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

work around GCC 6~11 ADL bug #3993

Merged
merged 1 commit into from
Apr 12, 2023

Conversation

pgroke-dt
Copy link
Contributor

see https://gcc.gnu.org/bugzilla/show_bug.cgi?id=51577
ADL seems to work properly when we do the SFINAE check via the return type, but not when using a dummy template parameter

fix #3992

@google-cla
Copy link

google-cla bot commented Aug 31, 2022

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

// ADL (possibly involving implicit conversions).
// (Use SFINAE via return type, because it seems GCC < 12 doesn't handle name
// lookup properly when we do it in the template parameter list.)
static decltype(std::declval<std::ostream&>() << std::declval<const T&>(),
Copy link
Contributor

Choose a reason for hiding this comment

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

Nitpick:
Lets write this with trailing retrun type so we have access to the variables and can avoid std::declval:
static auto PrintValue(const T& value, ::std::ostream* os) -> decltype((void)(*os << value)) { ... }

@asoffer
Copy link
Contributor

asoffer commented Sep 14, 2022

Generally looks good. Please sign the CLA, and we'll get this merged.

@pgroke-dt
Copy link
Contributor Author

The CLA should be signed, but somehow it still doesn't work. The issue is being discussed here: google/closure-compiler#3988
In the meantime I will update the PR to use trailing return type as you requested.

see https://gcc.gnu.org/bugzilla/show_bug.cgi?id=51577
ADL seems to work properly when we do the SFINAE check via the return type, but not when using a dummy template parameter

fix google#3992
@pgroke-dt
Copy link
Contributor Author

@asoffer Sorry for taking so long, but the CLA check is finally green now.

@pgroke-dt
Copy link
Contributor Author

@asoffer Hi. I made the changes you requested and the CLA check is green. Is there anything blocking us from moving forward with this?

@pgroke-dt
Copy link
Contributor Author

@asoffer @derekmauro I'd really like to proceed with this. Please let me know if there's something I need to do before this can be merged.

@Krzmbrzl
Copy link
Contributor

@asoffer @derekmauro @dinord could someone of you please look into this? This bug is what is keeping me from moving to following HEAD of googletest in my projects as they fail to compile 👀

@bencsikandrei
Copy link

is there any news on this one?

@Krzmbrzl
Copy link
Contributor

Unfortunately not and I have the feeling that my pings to @asoffer @derekmauro and @dinord didn't get through 🤷

@copybara-service copybara-service bot merged commit 12a5852 into google:main Apr 12, 2023
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.

Work around GCC name lookup bug with GCC 6~11 (StreamPrinter::PrintValue)
4 participants