r217302 - Add error, recovery and fixit for "~A::A() {...}".

David Majnemer david.majnemer at gmail.com
Thu Jan 15 00:16:13 PST 2015


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.

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/20150115/449c2f1d/attachment.html>


More information about the cfe-commits mailing list