This is an archive of the discontinued LLVM Phabricator instance.

[libcxx] Fix -Wdeprecated warnings
AbandonedPublic

Authored by hintonda on Mar 16 2016, 11:34 AM.

Details

Summary

Fix -Wdeprecated warning for throw() specs and implicit copy
ctor's and assignmen oper's.

Diff Detail

Event Timeline

hintonda updated this revision to Diff 50841.Mar 16 2016, 11:34 AM
hintonda retitled this revision from to [libcxx] Fix -Wdeprecated warnings.
hintonda updated this object.
hintonda added reviewers: rsmith, howard.hinnant, logan.
hintonda added a subscriber: cfe-commits.
rsmith edited reviewers, added: mclow.lists, EricWF; removed: howard.hinnant, rsmith.Mar 18 2016, 7:50 AM
logan removed a reviewer: logan.Mar 19 2016, 8:05 AM
logan added a subscriber: logan.Mar 19 2016, 8:17 AM

The simple replacement from throw() to _NOEXCEPT looks fine.

But, why are you adding adding some copy constructors and assignment operators? May you briefly explain the reason?

Besides, I have some concern with the = default specifier in some C++98 headers, e.g. <typeinfo>. What will happen if the user is compiling with -std=c++98?

And, unfortunately, I am not familiar with libc++ code base. I think you will need approvals from @mclow.lists or @EricWF.

Implicit copy ctor's and and assignment operators are deprecated in c++11, which is why I added them as = default, but you raise a good point. Perhaps it would be better to wrap them in #ifdef's.

Just to clarify, they are deprecated with an explicit dtor's are provided.

mclow.lists added inline comments.Mar 19 2016, 1:59 PM
include/__bit_reference
75

Sadly, we don't get to say '=default', because we have clients that compile under C++03. Please use _LIBCPP_DEFAULT instead.

hintonda added a comment.EditedMar 19 2016, 2:18 PM

Okay, but should I fix the 146 cases of " = default;" that were already there? That's a lot of churn, so I don't want to step on anyone...

looks like that included comments, so might not be that big of an issue, though I won't be able to do it quickly with sed.

mclow.lists edited edge metadata.Mar 19 2016, 3:27 PM

Okay, but should I fix the 146 cases of " = default;" that were already there?

If there are = default in code that can be used from C++03, then they ought to be fixed.
Note that many of the ones that turn up in a text search are in the synopsis - not in the code.

One idea is to do this in two steps - do the throw() stuff in one patch, and the defaulted fns in a second one.

Yes, those were mostly in comments. I'd just done a simple find/grep that didn't take context into account.

I agree that multiple checkins would be best which will make it much easy to review.

Let's retire this patch, then.
I'll be watching for your replacement patches.

Will do -- moved and started new job, so got a bit behind.

Btw, I have a new checker that's under review that will take care of the noexcept for you. And I think someone else has something that might take care of the implicit copy/assignment.

So this should be trivial very soon.

hintonda abandoned this revision.Apr 13 2016, 9:31 AM