Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(1230)

Issue 1407042: PR7245: Make rvalue->reference copy errors into warnings.

Can't Edit
Can't Publish+Mail
Start Review
Created:
9 years, 5 months ago by Jeffrey Yasskin
Modified:
9 years, 5 months ago
Reviewers:
DougGregor, cfe-commits
Base URL:
https://llvm.org/svn/llvm-project/cfe/trunk/
Visibility:
Public.

Description

Adds a -Wrvalue-copy-ctor to control the extwarn. Still fails when instantiating the call causes other templates to fail to instantitate, but fixing that appears to be too tricky to be worth it.

Patch Set 1 #

Patch Set 2 : Add docs #

Total comments: 10

Patch Set 3 : Fix some wording #

Total comments: 10

Patch Set 4 : Fix dgregor's comments #

Unified diffs Side-by-side diffs Delta from patch set Stats (+102 lines, -16 lines) Patch
M docs/UsersManual.html View 2 3 1 chunk +36 lines, -0 lines 0 comments Download
M include/clang/Basic/DiagnosticGroups.td View 1 2 3 1 chunk +1 line, -0 lines 0 comments Download
M include/clang/Basic/DiagnosticSemaKinds.td View 1 2 3 2 chunks +11 lines, -0 lines 0 comments Download
M lib/Sema/Sema.h View 1 2 3 1 chunk +2 lines, -1 line 0 comments Download
M lib/Sema/SemaAccess.cpp View 1 2 3 2 chunks +7 lines, -4 lines 0 comments Download
M lib/Sema/SemaInit.cpp View 1 2 3 2 chunks +7 lines, -3 lines 0 comments Download
M test/CXX/dcl.decl/dcl.init/dcl.init.ref/p5-cxx03-extra-copy.cpp View 1 2 3 4 chunks +24 lines, -8 lines 0 comments Download
M test/CXX/dcl.decl/dcl.init/dcl.init.ref/p5-cxx0x-no-extra-copy.cpp View 1 chunk +14 lines, -0 lines 0 comments Download

Messages

Total messages: 5
chandlerc
Initial comments inline. Fundamentally, I think these warnings should be disabled by default. From your ...
9 years, 5 months ago (2010-05-31 06:28:15 UTC) #1
Jeffrey Yasskin
On 2010/05/31 06:28:15, chandlerc wrote: > Initial comments inline. > > Fundamentally, I think these ...
9 years, 5 months ago (2010-06-02 21:57:08 UTC) #2
Jeffrey Yasskin
I couldn't find a way to use this error in SFINAE, so I didn't bother ...
9 years, 5 months ago (2010-06-02 21:59:32 UTC) #3
DougGregor
Here's some example code that is well-formed C++98 (since the lack of a suitable copy ...
9 years, 5 months ago (2010-06-04 23:23:07 UTC) #4
Jeffrey Yasskin
9 years, 5 months ago (2010-06-07 06:44:43 UTC) #5
Thanks for the sample code. I've added it to the test. I'll commit once the svn
server is back up.

http://codereview.appspot.com/1407042/diff/8001/9002
File include/clang/Basic/DiagnosticGroups.td (right):

http://codereview.appspot.com/1407042/diff/8001/9002#newcode76
include/clang/Basic/DiagnosticGroups.td:76: def RvalueCopyCtor :
DiagGroup<"rvalue-copy-ctor">;
On 2010/06/04 23:23:08, doug.gregor wrote:
> I think that the name "rvalue-copy-ctor" for this warning group is somewhat
> misleading; this is about binding a reference to a temporary. I don't have any
> great ideas, but perhaps something like "-Wbind-to-temporary-copy"?

Works for me.

http://codereview.appspot.com/1407042/diff/8001/9003
File include/clang/Basic/DiagnosticSemaKinds.td (right):

http://codereview.appspot.com/1407042/diff/8001/9003#newcode492
include/clang/Basic/DiagnosticSemaKinds.td:492: NoSFINAE,
InGroup<RvalueCopyCtor>;
On 2010/06/04 23:23:08, doug.gregor wrote:
> how about "when binding a reference to a temporary"

Done.

http://codereview.appspot.com/1407042/diff/8001/9003#newcode756
include/clang/Basic/DiagnosticSemaKinds.td:756: "constructor when binding an
rvalue to a reference">,
On 2010/06/04 23:23:08, doug.gregor wrote:
> same comment: "when binding a reference to a temporary" ?

Done.

http://codereview.appspot.com/1407042/diff/8001/9005
File lib/Sema/SemaInit.cpp (right):

http://codereview.appspot.com/1407042/diff/8001/9005#newcode3327
lib/Sema/SemaInit.cpp:3327: S.Diag(Loc, IsExtraneousCopy
On 2010/06/04 23:23:08, doug.gregor wrote:
> Use "IsExtraneousCopy && !S.isSFINAEContext()" to keep the error within SFINAE
> context.

Done.

http://codereview.appspot.com/1407042/diff/8001/9005#newcode3334
lib/Sema/SemaInit.cpp:3334: return S.ExprError();
On 2010/06/04 23:23:08, doug.gregor wrote:
> You're going to need to make this:
> 
>   if (!IsExtraneousCopy || S.isSFINAEContext())
>     return S.ExprError();
>   return move(CurInit);
> 
> As it stands, you've downgraded the error to a warning, but the initialization
> code is still returning an error.

Done, and I'm running a bootstrap on the assumption that that's where this would
have broken.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b