From 45fa560c72441069d9e4eb1e66efd87349caa552 Mon Sep 17 00:00:00 2001 From: Douglas Gregor Date: Mon, 7 Nov 2011 20:56:01 +0000 Subject: [PATCH] When we notice that a member function is defined with "= delete" or "= default", make a note of which is used when creating the initial declaration. Previously, we would wait until later to handle default/delete as a definition, but this is too late: when adding the declaration, we already treated the declaration as "user-provided" when in fact it was merely "user-declared". Fixes PR10861 and PR10442, along with a bunch of FIXMEs. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@144011 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/clang/Parse/Parser.h | 4 ++- include/clang/Sema/DeclSpec.h | 32 ++++++++++++++++--- lib/AST/DeclCXX.cpp | 10 +++--- lib/Parse/ParseCXXInlineMethods.cpp | 8 +++-- lib/Parse/ParseDeclCXX.cpp | 16 ++++++---- lib/Parse/Parser.cpp | 2 +- lib/Sema/SemaDecl.cpp | 26 ++++++++++++--- lib/Sema/SemaTemplate.cpp | 2 +- test/CXX/basic/basic.types/p10.cpp | 1 + test/CXX/class/p6-0x.cpp | 15 +++++++++ .../dcl.decl/dcl.init/dcl.init.aggr/p1-0x.cpp | 12 +++++++ test/CXX/special/class.ctor/p5-0x.cpp | 14 +++----- test/CXX/special/class.temporary/p1.cpp | 3 +- 13 files changed, 105 insertions(+), 40 deletions(-) diff --git a/include/clang/Parse/Parser.h b/include/clang/Parse/Parser.h index dba7ffb478..1b369d0ed3 100644 --- a/include/clang/Parse/Parser.h +++ b/include/clang/Parse/Parser.h @@ -1134,7 +1134,9 @@ private: Decl *ParseCXXInlineMethodDef(AccessSpecifier AS, AttributeList *AccessAttrs, ParsingDeclarator &D, const ParsedTemplateInfo &TemplateInfo, - const VirtSpecifiers& VS, ExprResult& Init); + const VirtSpecifiers& VS, + FunctionDefinitionKind DefinitionKind, + ExprResult& Init); void ParseCXXNonStaticMemberInitializer(Decl *VarD); void ParseLexedAttributes(ParsingClass &Class); void ParseLexedAttribute(LateParsedAttribute &LA); diff --git a/include/clang/Sema/DeclSpec.h b/include/clang/Sema/DeclSpec.h index 4d40a3d291..7dc8f29455 100644 --- a/include/clang/Sema/DeclSpec.h +++ b/include/clang/Sema/DeclSpec.h @@ -1379,6 +1379,15 @@ struct DeclaratorChunk { }; +/// \brief Described the kind of function definition (if any) provided for +/// a function. +enum FunctionDefinitionKind { + FDK_Declaration, + FDK_Definition, + FDK_Defaulted, + FDK_Deleted +}; + /// Declarator - Information about one declarator, including the parsed type /// information and the identifier. When the declarator is fully formed, this /// is turned into the appropriate Decl object. @@ -1434,8 +1443,11 @@ private: /// GroupingParens - Set by Parser::ParseParenDeclarator(). bool GroupingParens : 1; - /// FunctionDefinition - Is this Declarator for a function or member defintion - bool FunctionDefinition : 1; + /// FunctionDefinition - Is this Declarator for a function or member + /// definition and, if so, what kind? + /// + /// Actually a FunctionDefinitionKind. + unsigned FunctionDefinition : 2; // Redeclaration - Is this Declarator is a redeclaration. bool Redeclaration : 1; @@ -1465,7 +1477,8 @@ public: Declarator(const DeclSpec &ds, TheContext C) : DS(ds), Range(ds.getSourceRange()), Context(C), InvalidType(DS.getTypeSpecType() == DeclSpec::TST_error), - GroupingParens(false), FunctionDefinition(false), Redeclaration(false), + GroupingParens(false), FunctionDefinition(FDK_Declaration), + Redeclaration(false), Attrs(ds.getAttributePool().getFactory()), AsmLabel(0), InlineParamsUsed(false), Extension(false) { } @@ -1827,8 +1840,17 @@ public: SourceLocation getEllipsisLoc() const { return EllipsisLoc; } void setEllipsisLoc(SourceLocation EL) { EllipsisLoc = EL; } - void setFunctionDefinition(bool Val) { FunctionDefinition = Val; } - bool isFunctionDefinition() const { return FunctionDefinition; } + void setFunctionDefinitionKind(FunctionDefinitionKind Val) { + FunctionDefinition = Val; + } + + bool isFunctionDefinition() const { + return getFunctionDefinitionKind() != FDK_Declaration; + } + + FunctionDefinitionKind getFunctionDefinitionKind() const { + return (FunctionDefinitionKind)FunctionDefinition; + } void setRedeclaration(bool Val) { Redeclaration = Val; } bool isRedeclaration() const { return Redeclaration; } diff --git a/lib/AST/DeclCXX.cpp b/lib/AST/DeclCXX.cpp index b8c20e4b06..b86cf28118 100644 --- a/lib/AST/DeclCXX.cpp +++ b/lib/AST/DeclCXX.cpp @@ -451,8 +451,7 @@ void CXXRecordDecl::addedMember(Decl *D) { // -- class X has no virtual functions [...] data().HasTrivialCopyAssignment = false; data().HasTrivialMoveAssignment = false; - // FIXME: Destructor? - + // C++0x [class]p7: // A standard-layout class is a class that: [...] // -- has no virtual functions @@ -574,9 +573,10 @@ NotASpecialMember:; // This bit is the C++03 POD bit, not the 0x one. data().PlainOldData = false; - // C++0x [class.dtor]p5: - // A destructor is trivial if it is not user-provided and [...] - if (DD->isUserProvided()) + // C++11 [class.dtor]p5: + // A destructor is trivial if it is not user-provided and if + // -- the destructor is not virtual. + if (DD->isUserProvided() || DD->isVirtual()) data().HasTrivialDestructor = false; return; diff --git a/lib/Parse/ParseCXXInlineMethods.cpp b/lib/Parse/ParseCXXInlineMethods.cpp index 04c05d0cc3..65f7f56fe9 100644 --- a/lib/Parse/ParseCXXInlineMethods.cpp +++ b/lib/Parse/ParseCXXInlineMethods.cpp @@ -24,8 +24,10 @@ using namespace clang; Decl *Parser::ParseCXXInlineMethodDef(AccessSpecifier AS, AttributeList *AccessAttrs, ParsingDeclarator &D, - const ParsedTemplateInfo &TemplateInfo, - const VirtSpecifiers& VS, ExprResult& Init) { + const ParsedTemplateInfo &TemplateInfo, + const VirtSpecifiers& VS, + FunctionDefinitionKind DefinitionKind, + ExprResult& Init) { assert(D.isFunctionDeclarator() && "This isn't a function declarator!"); assert((Tok.is(tok::l_brace) || Tok.is(tok::colon) || Tok.is(tok::kw_try) || Tok.is(tok::equal)) && @@ -36,7 +38,7 @@ Decl *Parser::ParseCXXInlineMethodDef(AccessSpecifier AS, TemplateInfo.TemplateParams ? TemplateInfo.TemplateParams->size() : 0); Decl *FnD; - D.setFunctionDefinition(true); + D.setFunctionDefinitionKind(DefinitionKind); if (D.getDeclSpec().isFriendSpecified()) FnD = Actions.ActOnFriendFunctionDecl(getCurScope(), D, move(TemplateParams)); diff --git a/lib/Parse/ParseDeclCXX.cpp b/lib/Parse/ParseDeclCXX.cpp index 721d185f53..04c94a053d 100644 --- a/lib/Parse/ParseDeclCXX.cpp +++ b/lib/Parse/ParseDeclCXX.cpp @@ -1779,25 +1779,27 @@ void Parser::ParseCXXClassMemberDeclaration(AccessSpecifier AS, HasInitializer = true; } - bool IsDefinition = false; + FunctionDefinitionKind DefinitionKind = FDK_Declaration; // function-definition: // // In C++11, a non-function declarator followed by an open brace is a // braced-init-list for an in-class member initialization, not an // erroneous function definition. if (Tok.is(tok::l_brace) && !getLang().CPlusPlus0x) { - IsDefinition = true; + DefinitionKind = FDK_Definition; } else if (DeclaratorInfo.isFunctionDeclarator()) { if (Tok.is(tok::l_brace) || Tok.is(tok::colon) || Tok.is(tok::kw_try)) { - IsDefinition = true; + DefinitionKind = FDK_Definition; } else if (Tok.is(tok::equal)) { const Token &KW = NextToken(); - if (KW.is(tok::kw_default) || KW.is(tok::kw_delete)) - IsDefinition = true; + if (KW.is(tok::kw_default)) + DefinitionKind = FDK_Defaulted; + else if (KW.is(tok::kw_delete)) + DefinitionKind = FDK_Deleted; } } - if (IsDefinition) { + if (DefinitionKind) { if (!DeclaratorInfo.isFunctionDeclarator()) { Diag(Tok, diag::err_func_def_no_params); ConsumeBrace(); @@ -1825,7 +1827,7 @@ void Parser::ParseCXXClassMemberDeclaration(AccessSpecifier AS, Decl *FunDecl = ParseCXXInlineMethodDef(AS, AccessAttrs, DeclaratorInfo, TemplateInfo, - VS, Init); + VS, DefinitionKind, Init); for (unsigned i = 0, ni = LateParsedAttrs.size(); i < ni; ++i) { LateParsedAttrs[i]->setDecl(FunDecl); diff --git a/lib/Parse/Parser.cpp b/lib/Parse/Parser.cpp index 6e84e42968..b4d41ef482 100644 --- a/lib/Parse/Parser.cpp +++ b/lib/Parse/Parser.cpp @@ -876,7 +876,7 @@ Decl *Parser::ParseFunctionDefinition(ParsingDeclarator &D, ParseScope BodyScope(this, Scope::FnScope|Scope::DeclScope); Scope *ParentScope = getCurScope()->getParent(); - D.setFunctionDefinition(true); + D.setFunctionDefinitionKind(FDK_Definition); Decl *DP = Actions.HandleDeclarator(ParentScope, D, move(TemplateParameterLists)); D.complete(DP); diff --git a/lib/Sema/SemaDecl.cpp b/lib/Sema/SemaDecl.cpp index 05f48707aa..d71cd5f7d5 100644 --- a/lib/Sema/SemaDecl.cpp +++ b/lib/Sema/SemaDecl.cpp @@ -3145,7 +3145,7 @@ static bool RebuildDeclaratorInCurrentInstantiation(Sema &S, Declarator &D, } Decl *Sema::ActOnDeclarator(Scope *S, Declarator &D) { - D.setFunctionDefinition(false); + D.setFunctionDefinitionKind(FDK_Declaration); return HandleDeclarator(S, D, MultiTemplateParamsArg(*this)); } @@ -4803,7 +4803,7 @@ Sema::ActOnFunctionDeclarator(Scope *S, Declarator &D, DeclContext *DC, // scope specifier, or is the object of a friend declaration, the // lexical context will be different from the semantic context. NewFD->setLexicalDeclContext(CurContext); - + // Match up the template parameter lists with the scope specifier, then // determine whether we have a template or a template specialization. bool Invalid = false; @@ -4995,10 +4995,26 @@ Sema::ActOnFunctionDeclarator(Scope *S, Declarator &D, DeclContext *DC, NewFD->setAccess(AS_public); } + // If a function is defined as defaulted or deleted, mark it as such now. + switch (D.getFunctionDefinitionKind()) { + case FDK_Declaration: + case FDK_Definition: + break; + + case FDK_Defaulted: + NewFD->setDefaulted(); + break; + + case FDK_Deleted: + NewFD->setDeletedAsWritten(); + break; + } + if (isa(NewFD) && DC == CurContext && D.isFunctionDefinition()) { - // A method is implicitly inline if it's defined in its class - // definition. + // C++ [class.mfct]p2: + // A member function may be defined (8.4) in its class definition, in + // which case it is an inline member function (7.1.2) NewFD->setImplicitlyInline(); } @@ -6885,7 +6901,7 @@ Decl *Sema::ActOnStartOfFunctionDef(Scope *FnBodyScope, assert(D.isFunctionDeclarator() && "Not a function declarator!"); Scope *ParentScope = FnBodyScope->getParent(); - D.setFunctionDefinition(true); + D.setFunctionDefinitionKind(FDK_Definition); Decl *DP = HandleDeclarator(ParentScope, D, MultiTemplateParamsArg(*this)); return ActOnStartOfFunctionDef(FnBodyScope, DP); diff --git a/lib/Sema/SemaTemplate.cpp b/lib/Sema/SemaTemplate.cpp index 2d7e85094a..8ec4c195f9 100644 --- a/lib/Sema/SemaTemplate.cpp +++ b/lib/Sema/SemaTemplate.cpp @@ -5219,7 +5219,7 @@ Decl *Sema::ActOnStartOfFunctionTemplateDef(Scope *FnBodyScope, Scope *ParentScope = FnBodyScope->getParent(); - D.setFunctionDefinition(true); + D.setFunctionDefinitionKind(FDK_Definition); Decl *DP = HandleDeclarator(ParentScope, D, move(TemplateParameterLists)); if (FunctionTemplateDecl *FunctionTemplate diff --git a/test/CXX/basic/basic.types/p10.cpp b/test/CXX/basic/basic.types/p10.cpp index 3b438d15f2..1f66e5522d 100644 --- a/test/CXX/basic/basic.types/p10.cpp +++ b/test/CXX/basic/basic.types/p10.cpp @@ -18,6 +18,7 @@ struct UserProvDtor { constexpr UserProvDtor(); // expected-error {{non-literal type 'UserProvDtor' cannot have constexpr members}} ~UserProvDtor(); // expected-note {{has a user-provided destructor}} }; + struct NonTrivDtor { constexpr NonTrivDtor(); // expected-error {{non-literal type 'NonTrivDtor' cannot have constexpr members}} virtual ~NonTrivDtor() = default; // expected-note {{has a non-trivial destructor}} diff --git a/test/CXX/class/p6-0x.cpp b/test/CXX/class/p6-0x.cpp index 3384af09c7..f2cf482821 100644 --- a/test/CXX/class/p6-0x.cpp +++ b/test/CXX/class/p6-0x.cpp @@ -13,3 +13,18 @@ static_assert(!__is_trivial(NonTrivial2), "NonTrivial2 is trivial"); static_assert(!__is_trivial(NonTrivial3), "NonTrivial3 is trivial"); static_assert(!__is_trivial(NonTrivial4), "NonTrivial4 is trivial"); static_assert(!__is_trivial(NonTrivial5), "NonTrivial5 is trivial"); + +struct Trivial2 { + Trivial2() = default; + Trivial2(const Trivial2 &) = default; + Trivial2(Trivial2 &&) = default; + Trivial2 &operator=(const Trivial2 &) = default; + Trivial2 &operator=(Trivial2 &) = default; + ~Trivial2() = default; +}; + +class NonTrivial6 { ~NonTrivial6(); }; + +NonTrivial6::~NonTrivial6() = default; + +static_assert(!__is_trivial(NonTrivial6), "NonTrivial6 is trivial"); diff --git a/test/CXX/dcl.decl/dcl.init/dcl.init.aggr/p1-0x.cpp b/test/CXX/dcl.decl/dcl.init/dcl.init.aggr/p1-0x.cpp index 13b02c014e..ac8794f455 100644 --- a/test/CXX/dcl.decl/dcl.init/dcl.init.aggr/p1-0x.cpp +++ b/test/CXX/dcl.decl/dcl.init/dcl.init.aggr/p1-0x.cpp @@ -64,3 +64,15 @@ struct NonAggr6 { int n; }; NonAggr6 na6 = { 42 }; // expected-error {{non-aggregate type 'NonAggr6'}} + +struct DefaultedAggr { + int n; + + DefaultedAggr() = default; + DefaultedAggr(const DefaultedAggr &) = default; + DefaultedAggr(DefaultedAggr &&) = default; + DefaultedAggr &operator=(const DefaultedAggr &) = default; + DefaultedAggr &operator=(DefaultedAggr &) = default; + ~DefaultedAggr() = default; +}; +DefaultedAggr da = { 42 } ; diff --git a/test/CXX/special/class.ctor/p5-0x.cpp b/test/CXX/special/class.ctor/p5-0x.cpp index de2dea5be1..c8d206ae37 100644 --- a/test/CXX/special/class.ctor/p5-0x.cpp +++ b/test/CXX/special/class.ctor/p5-0x.cpp @@ -48,10 +48,8 @@ class Deleted3a { const int a; }; // expected-note {{here}} \ Deleted3a d3a; // expected-error {{deleted constructor}} class Deleted3b { const DefaultedDefCtor1 a[42]; }; // expected-note {{here}} Deleted3b d3b; // expected-error {{deleted constructor}} -// FIXME: clang implements the pre-FDIS rule, under which DefaultedDefCtor2's -// default constructor is user-provided. -class Deleted3c { const DefaultedDefCtor2 a; }; // desired-note {{here}} -Deleted3c d3c; // desired-error {{deleted constructor}} +class Deleted3c { const DefaultedDefCtor2 a; }; // expected-note {{deleted}} +Deleted3c d3c; // expected-error {{deleted constructor}} class NotDeleted3a { const int a = 0; }; NotDeleted3a nd3a; class NotDeleted3b { const DefaultedDefCtor1 a[42] = {}; }; @@ -159,11 +157,7 @@ static_assert(!__has_trivial_constructor(NonTrivialDefCtor6), "NonTrivialDefCtor // Otherwise, the default constructor is non-trivial. class Trivial2 { Trivial2() = delete; }; -//static_assert(__has_trivial_constructor(Trivial2), "NonTrivialDefCtor2 is trivial"); -// FIXME: clang implements the pre-FDIS rule, under which this class is non-trivial. -static_assert(!__has_trivial_constructor(Trivial2), "NonTrivialDefCtor2 is trivial"); +static_assert(__has_trivial_constructor(Trivial2), "NonTrivialDefCtor2 is trivial"); class Trivial3 { Trivial3() = default; }; -//static_assert(__has_trivial_constructor(Trivial3), "NonTrivialDefCtor3 is trivial"); -// FIXME: clang implements the pre-FDIS rule, under which this class is non-trivial. -static_assert(!__has_trivial_constructor(Trivial3), "NonTrivialDefCtor3 is trivial"); +static_assert(__has_trivial_constructor(Trivial3), "NonTrivialDefCtor3 is trivial"); diff --git a/test/CXX/special/class.temporary/p1.cpp b/test/CXX/special/class.temporary/p1.cpp index 384b1f89fd..4f6ac0a002 100644 --- a/test/CXX/special/class.temporary/p1.cpp +++ b/test/CXX/special/class.temporary/p1.cpp @@ -31,8 +31,7 @@ namespace test1 { void test() { A a; - // FIXME: this error about variadics is bogus - foo(a); // expected-error {{calling a private constructor of class 'test1::A'}} expected-error {{cannot pass object of non-trivial type 'test1::A' through variadic function}} + foo(a); // expected-error {{calling a private constructor of class 'test1::A'}} } }