r217302 - Add error, recovery and fixit for "~A::A() {...}".
Nico Weber
thakis at chromium.org
Wed Jan 14 16:57:16 PST 2015
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 <richard at metafoo.co.uk>
wrote:
> On Sun, Jan 4, 2015 at 2:26 PM, Nico Weber <thakis at chromium.org> wrote:
>
>> Sorry for the somewhat delayed review. Two points below.
>>
>> On Fri, Sep 5, 2014 at 7:06 PM, Richard Smith <richard-llvm at metafoo.co.uk
>> > 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
>>> cfe-commits at cs.uiuc.edu
>>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>>>
>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20150114/a454b8e7/attachment.html>
More information about the cfe-commits
mailing list