Page MenuHomePhabricator

[libc++] Fix -Wdeprecated-copy warnings in __bit_reference
AbandonedPublic

Authored by echristo on Dec 11 2019, 11:37 PM.

Details

Summary

Add a couple of default copy constructors to fix the warning.

Diff Detail

Event Timeline

echristo created this revision.Dec 11 2019, 11:37 PM
dblaikie accepted this revision.Dec 11 2019, 11:47 PM

Sounds good to me.

(my thought process, for posterity (beyond the fact that this code change makes explicit what was already happening/it doesn't change behavior): copy constructing a reference binds the new copy to the same entity as the original. This is different from copy assigning, which doesn't bind/rebind, but instead changes the value of the object referenced by the LHS reference to match the value of the object referenced by the RHS reference - so it seems good/proper that these types would have a non-trivial copy assignment, while wanting to opt back into the default copy construction)

This revision is now accepted and ready to land.Dec 11 2019, 11:47 PM
echristo updated this revision to Diff 233518.Dec 11 2019, 11:47 PM

clang-format changed lines.

Unit tests: pass. 60748 tests passed, 0 failed and 726 were skipped.

clang-format: pass.

Build artifacts: console-log.txt, CMakeCache.txt, test-results.xml, diff.json

This revision was automatically updated to reflect the committed changes.
echristo reopened this revision.Dec 12 2019, 12:24 AM

Reopening this.

Caused some failures due to public/private and the function was actually called in a number of places where it would need to be public. I'm not sure if we wanted to make this public... or do something else so reopening :)

This revision is now accepted and ready to land.Dec 12 2019, 12:24 AM

It appears that we have multiple things in review here.
See also D71096.
Also, is this in response to https://bugs.llvm.org/show_bug.cgi?id=44145 ? (which was closed as 'works for me')

echristo abandoned this revision.Dec 12 2019, 1:38 PM

Abandoning this one in favor of maskray's revision.

It appears that we have multiple things in review here.
See also D71096.
Also, is this in response to https://bugs.llvm.org/show_bug.cgi?id=44145 ? (which was closed as 'works for me')

It is, but I'm still seeing it on every build so getting D71096 approved would be great. Thanks :)