<div dir="ltr"><div class="gmail_extra">Sorry for the somewhat delayed review. Two points below.</div><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Sep 5, 2014 at 7:06 PM, Richard Smith <span dir="ltr"><<a href="mailto:richard-llvm@metafoo.co.uk" target="_blank">richard-llvm@metafoo.co.uk</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">Author: rsmith<br>
Date: Fri Sep 5 21:06:12 2014<br>
New Revision: 217302<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=217302&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=217302&view=rev</a><br>
Log:<br>
Add error, recovery and fixit for "~A::A() {...}".<br>
<br>
Modified:<br>
cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td<br>
cfe/trunk/lib/Parse/ParseExprCXX.cpp<br>
cfe/trunk/test/FixIt/fixit.cpp<br>
cfe/trunk/test/Parser/cxx-class.cpp<br>
<br>
Modified: cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td?rev=217302&r1=217301&r2=217302&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td?rev=217302&r1=217301&r2=217302&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td (original)<br>
+++ cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td Fri Sep 5 21:06:12 2014<br>
@@ -492,6 +492,8 @@ def err_constructor_bad_name : Error<<br>
"missing return type for function %0; did you mean the constructor name %1?">;<br>
def err_destructor_tilde_identifier : Error<<br>
"expected a class name after '~' to name a destructor">;<br>
+def err_destructor_tilde_scope : Error<<br>
+ "'~' in destructor name should be after nested name specifier">;<br>
def err_destructor_template_id : Error<<br>
"destructor name %0 does not refer to a template">;<br>
def err_default_arg_unparsed : Error<<br>
<br>
Modified: cfe/trunk/lib/Parse/ParseExprCXX.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseExprCXX.cpp?rev=217302&r1=217301&r2=217302&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseExprCXX.cpp?rev=217302&r1=217301&r2=217302&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/lib/Parse/ParseExprCXX.cpp (original)<br>
+++ cfe/trunk/lib/Parse/ParseExprCXX.cpp Fri Sep 5 21:06:12 2014<br>
@@ -2452,10 +2452,29 @@ bool Parser::ParseUnqualifiedId(CXXScope<br>
return true;<br>
}<br>
<br>
+ // If the user wrote ~T::T, correct it to T::~T.<br>
+ if (!TemplateSpecified && NextToken().is(tok::coloncolon)) {<br>
+ if (SS.isSet()) {<br>
+ AnnotateScopeToken(SS, /*NewAnnotation*/true);<br>
+ SS.clear();<br>
+ }<br>
+ if (ParseOptionalCXXScopeSpecifier(SS, ObjectType, EnteringContext))<br>
+ return true;<br></blockquote><div><br></div><div>This scope spec never reaches a DeclaratorScopeObj. Normally, ParseDirectDeclarator() calls DeclScopeObj.EnterDeclaratorScope() if Actions.ShouldEnterDeclaratorScope(), but this isn't done after calling ParseUnqualifiedId(). As a consequence, this parses:</div><div><br></div><div><div> struct B {</div><div> class F {};</div><div> ~B() throw(F);</div><div> };</div><div> B::~B() throw(F) {}</div></div><div><br></div><div>While this doesn't:</div><div><br></div><div><div> struct B {</div><div> class F {};</div><div> ~B() throw(F);</div><div> };</div></div><div> ~B::B() throw(F) {} // Complains about not knowing F, suggests B::F<br></div><div> <br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
+ if (Tok.isNot(tok::identifier) || NextToken().is(tok::coloncolon)) {<br>
+ Diag(TildeLoc, diag::err_destructor_tilde_scope);<br>
+ return true;<br>
+ }<br>
+<br>
+ // Recover as if the tilde had been written before the identifier.<br></blockquote><div><br></div><div>This angers and confuses sema if getDestructorName() below is called for an incomplete type:</div><div><br></div><div><div> struct B;</div><div> ~B::B() {}</div></div><div><br></div><div>=> Assertion failed: ((!isa<TagDecl>(LookupCtx) || LookupCtx->isDependentContext() || cast<TagDecl>(LookupCtx)->isCompleteDefinition() || cast<TagDecl>(LookupCtx)->isBeingDefined()) && "Declaration context must already be complete!"), function LookupQualifiedName, file /Users/thakis/src/llvm-rw/tools/clang/lib/Sema/SemaLookup.cpp, line 1595.<br></div><div><div>6 clang::Sema::LookupQualifiedName()</div><div>7 clang::Sema::getDestructorName()</div><div>8 clang::Parser::ParseUnqualifiedId()</div><div>9 clang::Parser::ParseDirectDeclarator()</div></div><div><br></div><div>Calling ActOnCXXEnterDeclaratorScope() (via DeclaratorScopeObj) would fix this as it checks for complete types in the declarator scope, but it's not obvious to me how to do this from here – and doing it in ParseDirectDeclarator() after this function has been called would mean it'd happen after this function calls getDestructorName().</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
+ Diag(TildeLoc, diag::err_destructor_tilde_scope)<br>
+ << FixItHint::CreateRemoval(TildeLoc)<br>
+ << FixItHint::CreateInsertion(Tok.getLocation(), "~");<br>
+ }<br>
+<br>
// Parse the class-name (or template-name in a simple-template-id).<br>
IdentifierInfo *ClassName = Tok.getIdentifierInfo();<br>
SourceLocation ClassNameLoc = ConsumeToken();<br>
-<br>
+<br>
if (TemplateSpecified || Tok.is(tok::less)) {<br>
Result.setDestructorName(TildeLoc, ParsedType(), ClassNameLoc);<br>
return ParseUnqualifiedIdTemplateId(SS, TemplateKWLoc,<br>
@@ -2463,7 +2482,7 @@ bool Parser::ParseUnqualifiedId(CXXScope<br>
EnteringContext, ObjectType,<br>
Result, TemplateSpecified);<br>
}<br>
-<br>
+<br>
// Note that this is a destructor name.<br>
ParsedType Ty = Actions.getDestructorName(TildeLoc, *ClassName,<br>
ClassNameLoc, getCurScope(),<br>
<br>
Modified: cfe/trunk/test/FixIt/fixit.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/FixIt/fixit.cpp?rev=217302&r1=217301&r2=217302&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/FixIt/fixit.cpp?rev=217302&r1=217301&r2=217302&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/test/FixIt/fixit.cpp (original)<br>
+++ cfe/trunk/test/FixIt/fixit.cpp Fri Sep 5 21:06:12 2014<br>
@@ -308,6 +308,13 @@ namespace dtor_fixit {<br>
~bar() { } // expected-error {{expected the class name after '~' to name a destructor}}<br>
// CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:6-[[@LINE-1]]:9}:"foo"<br>
};<br>
+<br>
+ class bar {<br>
+ ~bar();<br>
+ };<br>
+ ~bar::bar() {} // expected-error {{'~' in destructor name should be after nested name specifier}}<br>
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:4}:""<br>
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:9-[[@LINE-2]]:9}:"~"<br>
}<br>
<br>
namespace PR5066 {<br>
<br>
Modified: cfe/trunk/test/Parser/cxx-class.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Parser/cxx-class.cpp?rev=217302&r1=217301&r2=217302&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Parser/cxx-class.cpp?rev=217302&r1=217301&r2=217302&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/test/Parser/cxx-class.cpp (original)<br>
+++ cfe/trunk/test/Parser/cxx-class.cpp Fri Sep 5 21:06:12 2014<br>
@@ -139,6 +139,20 @@ namespace CtorErrors {<br>
};<br>
}<br>
<br>
+namespace DtorErrors {<br>
+ struct A { ~A(); } a;<br>
+ ~A::A() {} // expected-error {{'~' in destructor name should be after nested name specifier}} expected-note {{previous}}<br>
+ A::~A() {} // expected-error {{redefinition}}<br>
+<br>
+ struct B { ~B(); } *b;<br>
+ DtorErrors::~B::B() {} // expected-error {{'~' in destructor name should be after nested name specifier}}<br>
+<br>
+ void f() {<br>
+ a.~A::A(); // expected-error {{'~' in destructor name should be after nested name specifier}}<br>
+ b->~DtorErrors::~B::B(); // expected-error {{'~' in destructor name should be after nested name specifier}}<br>
+ }<br>
+}<br>
+<br>
namespace BadFriend {<br>
struct A {<br>
friend int : 3; // expected-error {{friends can only be classes or functions}}<br>
<br>
<br>
_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@cs.uiuc.edu">cfe-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><br>
</blockquote></div><br></div></div>