Skip to content

Fix Clang's -Wdeprecated-copy warnings in C++20#2758

Closed
Quuxplusone wants to merge 2 commits intogoogle:masterfrom
Quuxplusone:cxx20
Closed

Fix Clang's -Wdeprecated-copy warnings in C++20#2758
Quuxplusone wants to merge 2 commits intogoogle:masterfrom
Quuxplusone:cxx20

Conversation

@Quuxplusone
Copy link
Copy Markdown
Contributor

Fixes #1305.
(I think. Hopefully you've got a buildbot that will tell me if this breaks something.)

@googlebot
Copy link
Copy Markdown

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

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here with @googlebot I signed it! and we'll verify it.


What to do if you already signed the CLA

Individual signers
Corporate signers

ℹ️ Googlers: Go here for more info.

@Quuxplusone
Copy link
Copy Markdown
Contributor Author

@googlebot I signed it!

@googlebot
Copy link
Copy Markdown

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

@Quuxplusone Quuxplusone force-pushed the cxx20 branch 2 times, most recently from b53c283 to 29fc7f0 Compare March 23, 2020 17:00
This is a more aggressively "modern" approach than my first few attempts.
Apparently we can't just follow the Rule of Zero for the `Ne` matcher in
particular, because if we do, one of the unit tests hits
"recursive template instantiation exceeded maximum depth of 256"
on Clang.

Fixes google#1305.
… const/reference members.

These classes are already non-assignable; we don't need to say so explicitly.
@Quuxplusone
Copy link
Copy Markdown
Contributor Author

@gennadiycivil: Please commit this patch.

@aardappel: Here's the GTest/GMock counterpart to google/flatbuffers#5829 . Do you have any sway over here?

@aardappel
Copy link
Copy Markdown

@Quuxplusone I'm afraid I don't :)

@zhangxy988 zhangxy988 self-requested a review March 27, 2020 20:10
@zhangxy988 zhangxy988 self-assigned this Mar 27, 2020
Copy link
Copy Markdown
Contributor

@zhangxy988 zhangxy988 left a comment

Choose a reason for hiding this comment

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

We don't want any macros, so please replace GTEST_DISALLOW_ASSIGN_BUT_DEFAULT_COPY_ by spelling it out. Thanks!

@Quuxplusone
Copy link
Copy Markdown
Contributor Author

Quuxplusone commented Mar 27, 2020

We don't want any macros, so please replace GTEST_DISALLOW_ASSIGN_BUT_DEFAULT_COPY_ by spelling it out. Thanks!

@zhangxy988: That's awesome news. Would you mind if I submitted another PR to remove these macros first (i.e. manually preprocess these macros throughout the codebase, without changing any behavior), and then submitted a followup PR with just localized fixes for the Clang C++20 warnings? I'll work on that tonight and add you as a reviewer on the PR(s).

EDIT: well, I'll probably aim to make it one PR with two commits in it

@Quuxplusone
Copy link
Copy Markdown
Contributor Author

@zhangxy988: I've opened #2772 and #2773. Once those are both in, then I'll submit a PR for the macro removal and to add -Wdeprecated to the Travis build. (I just accidentally opened that as #2774, but I don't think it's worth looking at #2774 until #2772 and #2773 are both in, so I closed #2774 for now.)

@Quuxplusone
Copy link
Copy Markdown
Contributor Author

Abandoned in favor of #2815.

@Quuxplusone Quuxplusone deleted the cxx20 branch April 17, 2020 17:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Clang warning with -Wdeprecated in MATCHER_P generated matchers

4 participants