r217302 - Add error, recovery and fixit for "~A::A() {...}".
Nico Weber
thakis at chromium.org
Thu Jan 29 20:07:08 PST 2015
On Thu, Jan 15, 2015 at 12:16 AM, David Majnemer <david.majnemer at gmail.com>
wrote:
> There seems to be an issue when the class type has never been declared at
> all:
>
> ~B::B() {}
>
> This triggers:
> lib/Sema/SemaCXXScopeSpec.cpp:961: bool
> clang::Sema::ShouldEnterDeclaratorScope(clang::Scope*, const
> clang::CXXScopeSpec&): Assertion `SS.isSet() && "Parser passed invalid
> CXXScopeSpec."' failed.
>
Thanks, I fixed this in r227555.
>
> On Wed, Jan 14, 2015 at 4:57 PM, Nico Weber <thakis at chromium.org> wrote:
>
>> 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
>>>>>
>>>>
>>>>
>>>
>>
>> _______________________________________________
>> 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/20150129/25e6195a/attachment.html>
More information about the cfe-commits
mailing list