On Sun, Jan 4, 2015 at 2:26 PM, Nico Weber <[email protected]> wrote:
> Sorry for the somewhat delayed review. Two points below. > > On Fri, Sep 5, 2014 at 7:06 PM, Richard Smith <[email protected]> > wrote: > >> Author: rsmith >> Date: Fri Sep 5 21:06:12 2014 >> New Revision: 217302 >> >> URL: http://llvm.org/viewvc/llvm-project?rev=217302&view=rev >> Log: >> Add error, recovery and fixit for "~A::A() {...}". >> >> Modified: >> cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td >> cfe/trunk/lib/Parse/ParseExprCXX.cpp >> cfe/trunk/test/FixIt/fixit.cpp >> cfe/trunk/test/Parser/cxx-class.cpp >> >> Modified: cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td?rev=217302&r1=217301&r2=217302&view=diff >> >> ============================================================================== >> --- cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td (original) >> +++ cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td Fri Sep 5 >> 21:06:12 2014 >> @@ -492,6 +492,8 @@ def err_constructor_bad_name : Error< >> "missing return type for function %0; did you mean the constructor >> name %1?">; >> def err_destructor_tilde_identifier : Error< >> "expected a class name after '~' to name a destructor">; >> +def err_destructor_tilde_scope : Error< >> + "'~' in destructor name should be after nested name specifier">; >> def err_destructor_template_id : Error< >> "destructor name %0 does not refer to a template">; >> def err_default_arg_unparsed : Error< >> >> Modified: cfe/trunk/lib/Parse/ParseExprCXX.cpp >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseExprCXX.cpp?rev=217302&r1=217301&r2=217302&view=diff >> >> ============================================================================== >> --- cfe/trunk/lib/Parse/ParseExprCXX.cpp (original) >> +++ cfe/trunk/lib/Parse/ParseExprCXX.cpp Fri Sep 5 21:06:12 2014 >> @@ -2452,10 +2452,29 @@ bool Parser::ParseUnqualifiedId(CXXScope >> return true; >> } >> >> + // If the user wrote ~T::T, correct it to T::~T. >> + if (!TemplateSpecified && NextToken().is(tok::coloncolon)) { >> + if (SS.isSet()) { >> + AnnotateScopeToken(SS, /*NewAnnotation*/true); >> + SS.clear(); >> + } >> + if (ParseOptionalCXXScopeSpecifier(SS, ObjectType, >> EnteringContext)) >> + return true; >> > > 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: > > struct B { > class F {}; > ~B() throw(F); > }; > B::~B() throw(F) {} > > While this doesn't: > > struct B { > class F {}; > ~B() throw(F); > }; > ~B::B() throw(F) {} // Complains about not knowing F, suggests B::F > > >> + if (Tok.isNot(tok::identifier) || NextToken().is(tok::coloncolon)) >> { >> + Diag(TildeLoc, diag::err_destructor_tilde_scope); >> + return true; >> + } >> + >> + // Recover as if the tilde had been written before the identifier. >> > > This angers and confuses sema if getDestructorName() below is called for > an incomplete type: > > struct B; > ~B::B() {} > > => 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. > 6 clang::Sema::LookupQualifiedName() > 7 clang::Sema::getDestructorName() > 8 clang::Parser::ParseUnqualifiedId() > 9 clang::Parser::ParseDirectDeclarator() > > 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(). > Yuck, thanks. Fixed in r226067. For now I made us enter the scope twice (once within ParseUnqualifiedId and again once we're done parsing it), but ideas for better options would be welcome. > + Diag(TildeLoc, diag::err_destructor_tilde_scope) >> + << FixItHint::CreateRemoval(TildeLoc) >> + << FixItHint::CreateInsertion(Tok.getLocation(), "~"); >> + } >> + >> // Parse the class-name (or template-name in a simple-template-id). >> IdentifierInfo *ClassName = Tok.getIdentifierInfo(); >> SourceLocation ClassNameLoc = ConsumeToken(); >> - >> + >> if (TemplateSpecified || Tok.is(tok::less)) { >> Result.setDestructorName(TildeLoc, ParsedType(), ClassNameLoc); >> return ParseUnqualifiedIdTemplateId(SS, TemplateKWLoc, >> @@ -2463,7 +2482,7 @@ bool Parser::ParseUnqualifiedId(CXXScope >> EnteringContext, ObjectType, >> Result, TemplateSpecified); >> } >> - >> + >> // Note that this is a destructor name. >> ParsedType Ty = Actions.getDestructorName(TildeLoc, *ClassName, >> ClassNameLoc, >> getCurScope(), >> >> Modified: cfe/trunk/test/FixIt/fixit.cpp >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/FixIt/fixit.cpp?rev=217302&r1=217301&r2=217302&view=diff >> >> ============================================================================== >> --- cfe/trunk/test/FixIt/fixit.cpp (original) >> +++ cfe/trunk/test/FixIt/fixit.cpp Fri Sep 5 21:06:12 2014 >> @@ -308,6 +308,13 @@ namespace dtor_fixit { >> ~bar() { } // expected-error {{expected the class name after '~' to >> name a destructor}} >> // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:6-[[@LINE-1]]:9}:"foo" >> }; >> + >> + class bar { >> + ~bar(); >> + }; >> + ~bar::bar() {} // expected-error {{'~' in destructor name should be >> after nested name specifier}} >> + // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:4}:"" >> + // CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:9-[[@LINE-2]]:9}:"~" >> } >> >> namespace PR5066 { >> >> Modified: cfe/trunk/test/Parser/cxx-class.cpp >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Parser/cxx-class.cpp?rev=217302&r1=217301&r2=217302&view=diff >> >> ============================================================================== >> --- cfe/trunk/test/Parser/cxx-class.cpp (original) >> +++ cfe/trunk/test/Parser/cxx-class.cpp Fri Sep 5 21:06:12 2014 >> @@ -139,6 +139,20 @@ namespace CtorErrors { >> }; >> } >> >> +namespace DtorErrors { >> + struct A { ~A(); } a; >> + ~A::A() {} // expected-error {{'~' in destructor name should be after >> nested name specifier}} expected-note {{previous}} >> + A::~A() {} // expected-error {{redefinition}} >> + >> + struct B { ~B(); } *b; >> + DtorErrors::~B::B() {} // expected-error {{'~' in destructor name >> should be after nested name specifier}} >> + >> + void f() { >> + a.~A::A(); // expected-error {{'~' in destructor name should be >> after nested name specifier}} >> + b->~DtorErrors::~B::B(); // expected-error {{'~' in destructor name >> should be after nested name specifier}} >> + } >> +} >> + >> namespace BadFriend { >> struct A { >> friend int : 3; // expected-error {{friends can only be classes or >> functions}} >> >> >> _______________________________________________ >> cfe-commits mailing list >> [email protected] >> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits >> > >
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
