r217302 - Add error, recovery and fixit for "~A::A() {...}".
Nico Weber
thakis at chromium.org
Sun Jan 4 14:26:16 PST 2015
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().
> + 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/20150104/3260e9b0/attachment.html>
More information about the cfe-commits
mailing list