Thanks! One idea I thought about was to pass in DeclScopeObj to ParseUnqualifiedId(), but there's a bunch of other callers to ParseUnqualifiedId() so that's probably not much simpler.
On Wed, Jan 14, 2015 at 4:50 PM, Richard Smith <[email protected]> wrote: > 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
