From: DeLesley Hutchins Date: Thu, 16 Feb 2012 16:50:43 +0000 (+0000) Subject: Allow thread safety attributes on function definitions. X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=c24a2335677f3d1bd2cab1019ac445d650f52123;p=clang Allow thread safety attributes on function definitions. For compatibility with gcc, clang will now parse gcc attributes on function definitions, but issue a warning if the attribute is not a thread safety attribute. Warning controlled by -Wgcc-compat. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@150698 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/include/clang/Basic/DiagnosticGroups.td b/include/clang/Basic/DiagnosticGroups.td index 4924e85f2c..d4c733634b 100644 --- a/include/clang/Basic/DiagnosticGroups.td +++ b/include/clang/Basic/DiagnosticGroups.td @@ -364,6 +364,8 @@ def C99 : DiagGroup<"c99-extensions">; // A warning group for warnings about GCC extensions. def GNU : DiagGroup<"gnu", [GNUDesignator, VLA]>; +// A warning group for warnings about code that clang accepts but gcc doesn't. +def GccCompat : DiagGroup<"gcc-compat">; // A warning group for warnings about Microsoft extensions. def Microsoft : DiagGroup<"microsoft">; diff --git a/include/clang/Basic/DiagnosticParseKinds.td b/include/clang/Basic/DiagnosticParseKinds.td index 0955b59978..dcc6f697f8 100644 --- a/include/clang/Basic/DiagnosticParseKinds.td +++ b/include/clang/Basic/DiagnosticParseKinds.td @@ -151,6 +151,9 @@ def err_at_in_class : Error<"unexpected '@' in member specification">; def err_expected_fn_body : Error< "expected function body after function declarator">; +def warn_attribute_on_function_definition : Warning< + "GCC does not allow %0 attribute in this position on a function definition">, + InGroup; def err_expected_method_body : Error<"expected method body">; def err_invalid_token_after_toplevel_declarator : Error< "expected ';' after top level declarator">; diff --git a/include/clang/Parse/Parser.h b/include/clang/Parse/Parser.h index 0c076210a2..08af0b3437 100644 --- a/include/clang/Parse/Parser.h +++ b/include/clang/Parse/Parser.h @@ -1157,7 +1157,10 @@ private: ExprResult& Init); void ParseCXXNonStaticMemberInitializer(Decl *VarD); void ParseLexedAttributes(ParsingClass &Class); - void ParseLexedAttribute(LateParsedAttribute &LA); + void ParseLexedAttributeList(LateParsedAttrList &LAs, Decl *D, + bool EnterScope, bool OnDefinition); + void ParseLexedAttribute(LateParsedAttribute &LA, + bool EnterScope, bool OnDefinition); void ParseLexedMethodDeclarations(ParsingClass &Class); void ParseLexedMethodDeclaration(LateParsedMethodDeclaration &LM); void ParseLexedMethodDefs(ParsingClass &Class); @@ -1196,7 +1199,8 @@ private: AccessSpecifier AS = AS_none); Decl *ParseFunctionDefinition(ParsingDeclarator &D, - const ParsedTemplateInfo &TemplateInfo = ParsedTemplateInfo()); + const ParsedTemplateInfo &TemplateInfo = ParsedTemplateInfo(), + LateParsedAttrList *LateParsedAttrs = 0); void ParseKNRParamDeclarations(Declarator &D); // EndLoc, if non-NULL, is filled with the location of the last token of // the simple-asm. @@ -1642,7 +1646,7 @@ private: ForRangeInit *FRI = 0); Decl *ParseDeclarationAfterDeclarator(Declarator &D, const ParsedTemplateInfo &TemplateInfo = ParsedTemplateInfo()); - bool ParseAttributesAfterDeclarator(Declarator &D); + bool ParseAsmAttributesAfterDeclarator(Declarator &D); Decl *ParseDeclarationAfterDeclaratorAndAttributes(Declarator &D, const ParsedTemplateInfo &TemplateInfo = ParsedTemplateInfo()); Decl *ParseFunctionStatementBody(Decl *Decl, ParseScope &BodyScope); diff --git a/lib/Parse/ParseDecl.cpp b/lib/Parse/ParseDecl.cpp index 914a5acd09..0b98274002 100644 --- a/lib/Parse/ParseDecl.cpp +++ b/lib/Parse/ParseDecl.cpp @@ -132,14 +132,15 @@ void Parser::ParseGNUAttributes(ParsedAttributes &attrs, if (Tok.is(tok::l_paren)) { // handle "parameterized" attributes - if (LateAttrs && !ClassStack.empty() && - isAttributeLateParsed(*AttrName)) { - // Delayed parsing is only available for attributes that occur - // in certain locations within a class scope. + if (LateAttrs && isAttributeLateParsed(*AttrName)) { LateParsedAttribute *LA = new LateParsedAttribute(this, *AttrName, AttrNameLoc); LateAttrs->push_back(LA); - getCurrentClass().LateParsedDeclarations.push_back(LA); + + // Attributes in a class are parsed at the end of the class, along + // with other late-parsed declarations. + if (!ClassStack.empty()) + getCurrentClass().LateParsedDeclarations.push_back(LA); // consume everything up to and including the matching right parens ConsumeAndStoreUntil(tok::r_paren, LA->Toks, true, false); @@ -711,7 +712,7 @@ void Parser::LateParsedClass::ParseLexedAttributes() { } void Parser::LateParsedAttribute::ParseLexedAttributes() { - Self->ParseLexedAttribute(*this); + Self->ParseLexedAttribute(*this, true, false); } /// Wrapper class which calls ParseLexedAttribute, after setting up the @@ -736,12 +737,25 @@ void Parser::ParseLexedAttributes(ParsingClass &Class) { } } + +/// \brief Parse all attributes in LAs, and attach them to Decl D. +void Parser::ParseLexedAttributeList(LateParsedAttrList &LAs, Decl *D, + bool EnterScope, bool OnDefinition) { + for (unsigned i = 0, ni = LAs.size(); i < ni; ++i) { + LAs[i]->setDecl(D); + ParseLexedAttribute(*LAs[i], EnterScope, OnDefinition); + } + LAs.clear(); +} + + /// \brief Finish parsing an attribute for which parsing was delayed. /// This will be called at the end of parsing a class declaration /// for each LateParsedAttribute. We consume the saved tokens and /// create an attribute with the arguments filled in. We add this /// to the Attribute list for the decl. -void Parser::ParseLexedAttribute(LateParsedAttribute &LA) { +void Parser::ParseLexedAttribute(LateParsedAttribute &LA, + bool EnterScope, bool OnDefinition) { // Save the current token position. SourceLocation OrigLoc = Tok.getLocation(); @@ -752,17 +766,23 @@ void Parser::ParseLexedAttribute(LateParsedAttribute &LA) { // Consume the previously pushed token. ConsumeAnyToken(); + if (OnDefinition && !IsThreadSafetyAttribute(LA.AttrName.getName())) { + Diag(Tok, diag::warn_attribute_on_function_definition) + << LA.AttrName.getName(); + } + ParsedAttributes Attrs(AttrFactory); SourceLocation endLoc; // If the Decl is templatized, add template parameters to scope. - bool HasTemplateScope = LA.D && LA.D->isTemplateDecl(); + bool HasTemplateScope = EnterScope && LA.D && LA.D->isTemplateDecl(); ParseScope TempScope(this, Scope::TemplateParamScope, HasTemplateScope); if (HasTemplateScope) Actions.ActOnReenterTemplateScope(Actions.CurScope, LA.D); // If the Decl is on a function, add function parameters to the scope. - bool HasFunctionScope = LA.D && LA.D->isFunctionOrFunctionTemplate(); + bool HasFunctionScope = EnterScope && LA.D && + LA.D->isFunctionOrFunctionTemplate(); ParseScope FnScope(this, Scope::FnScope|Scope::DeclScope, HasFunctionScope); if (HasFunctionScope) Actions.ActOnReenterFunctionContext(Actions.CurScope, LA.D); @@ -1064,6 +1084,12 @@ Parser::DeclGroupPtrTy Parser::ParseDeclGroup(ParsingDeclSpec &DS, return DeclGroupPtrTy(); } + // Save late-parsed attributes for now; they need to be parsed in the + // appropriate function scope after the function Decl has been constructed. + LateParsedAttrList LateParsedAttrs; + if (D.isFunctionDeclarator()) + MaybeParseGNUAttributes(D, &LateParsedAttrs); + // Check to see if we have a function *definition* which must have a body. if (AllowFunctionDefinitions && D.isFunctionDeclarator() && // Look at the next token to make sure that this isn't a function @@ -1079,7 +1105,8 @@ Parser::DeclGroupPtrTy Parser::ParseDeclGroup(ParsingDeclSpec &DS, DS.ClearStorageClassSpecs(); } - Decl *TheDecl = ParseFunctionDefinition(D); + Decl *TheDecl = + ParseFunctionDefinition(D, ParsedTemplateInfo(), &LateParsedAttrs); return Actions.ConvertDeclToDeclGroup(TheDecl); } @@ -1096,7 +1123,7 @@ Parser::DeclGroupPtrTy Parser::ParseDeclGroup(ParsingDeclSpec &DS, } } - if (ParseAttributesAfterDeclarator(D)) + if (ParseAsmAttributesAfterDeclarator(D)) return DeclGroupPtrTy(); // C++0x [stmt.iter]p1: Check if we have a for-range-declarator. If so, we @@ -1117,6 +1144,8 @@ Parser::DeclGroupPtrTy Parser::ParseDeclGroup(ParsingDeclSpec &DS, SmallVector DeclsInGroup; Decl *FirstDecl = ParseDeclarationAfterDeclaratorAndAttributes(D); + if (LateParsedAttrs.size() > 0) + ParseLexedAttributeList(LateParsedAttrs, FirstDecl, true, false); D.complete(FirstDecl); if (FirstDecl) DeclsInGroup.push_back(FirstDecl); @@ -1185,7 +1214,7 @@ Parser::DeclGroupPtrTy Parser::ParseDeclGroup(ParsingDeclSpec &DS, /// Parse an optional simple-asm-expr and attributes, and attach them to a /// declarator. Returns true on an error. -bool Parser::ParseAttributesAfterDeclarator(Declarator &D) { +bool Parser::ParseAsmAttributesAfterDeclarator(Declarator &D) { // If a simple-asm-expr is present, parse it. if (Tok.is(tok::kw_asm)) { SourceLocation Loc; @@ -1227,7 +1256,7 @@ bool Parser::ParseAttributesAfterDeclarator(Declarator &D) { /// Decl *Parser::ParseDeclarationAfterDeclarator(Declarator &D, const ParsedTemplateInfo &TemplateInfo) { - if (ParseAttributesAfterDeclarator(D)) + if (ParseAsmAttributesAfterDeclarator(D)) return 0; return ParseDeclarationAfterDeclaratorAndAttributes(D, TemplateInfo); diff --git a/lib/Parse/ParseTemplate.cpp b/lib/Parse/ParseTemplate.cpp index 1222fb08f5..a30ef96d63 100644 --- a/lib/Parse/ParseTemplate.cpp +++ b/lib/Parse/ParseTemplate.cpp @@ -241,6 +241,10 @@ Parser::ParseSingleDeclarationAfterTemplate( return 0; } + LateParsedAttrList LateParsedAttrs; + if (DeclaratorInfo.isFunctionDeclarator()) + MaybeParseGNUAttributes(DeclaratorInfo, &LateParsedAttrs); + // If we have a declaration or declarator list, handle it. if (isDeclarationAfterDeclarator()) { // Parse this declaration. @@ -256,6 +260,8 @@ Parser::ParseSingleDeclarationAfterTemplate( // Eat the semi colon after the declaration. ExpectAndConsume(tok::semi, diag::err_expected_semi_declaration); + if (LateParsedAttrs.size() > 0) + ParseLexedAttributeList(LateParsedAttrs, ThisDecl, true, false); DeclaratorInfo.complete(ThisDecl); return ThisDecl; } @@ -270,7 +276,8 @@ Parser::ParseSingleDeclarationAfterTemplate( << FixItHint::CreateRemoval(DS.getStorageClassSpecLoc()); DS.ClearStorageClassSpecs(); } - return ParseFunctionDefinition(DeclaratorInfo, TemplateInfo); + return ParseFunctionDefinition(DeclaratorInfo, TemplateInfo, + &LateParsedAttrs); } if (DeclaratorInfo.isFunctionDeclarator()) diff --git a/lib/Parse/Parser.cpp b/lib/Parse/Parser.cpp index 4d8f474514..c797436948 100644 --- a/lib/Parse/Parser.cpp +++ b/lib/Parse/Parser.cpp @@ -821,7 +821,8 @@ Parser::ParseDeclarationOrFunctionDefinition(ParsedAttributes &attrs, /// decl-specifier-seq[opt] declarator function-try-block /// Decl *Parser::ParseFunctionDefinition(ParsingDeclarator &D, - const ParsedTemplateInfo &TemplateInfo) { + const ParsedTemplateInfo &TemplateInfo, + LateParsedAttrList *LateParsedAttrs) { // Poison the SEH identifiers so they are flagged as illegal in function bodies PoisonSEHIdentifiersRAIIObject PoisonSEHIdentifiers(*this, true); const DeclaratorChunk::FunctionTypeInfo &FTI = D.getFunctionTypeInfo(); @@ -844,7 +845,6 @@ Decl *Parser::ParseFunctionDefinition(ParsingDeclarator &D, if (FTI.isKNRPrototype()) ParseKNRParamDeclarations(D); - // We should have either an opening brace or, in a C++ constructor, // we may have a colon. if (Tok.isNot(tok::l_brace) && @@ -861,6 +861,19 @@ Decl *Parser::ParseFunctionDefinition(ParsingDeclarator &D, return 0; } + // Check to make sure that any normal attributes are allowed to be on + // a definition. Late parsed attributes are checked at the end. + if (Tok.isNot(tok::equal)) { + AttributeList *DtorAttrs = D.getAttributes(); + while (DtorAttrs) { + if (!IsThreadSafetyAttribute(DtorAttrs->getName()->getName())) { + Diag(DtorAttrs->getLoc(), diag::warn_attribute_on_function_definition) + << DtorAttrs->getName()->getName(); + } + DtorAttrs = DtorAttrs->getNext(); + } + } + // In delayed template parsing mode, for function template we consume the // tokens and store them for late parsing at the end of the translation unit. if (getLang().DelayedTemplateParsing && @@ -974,6 +987,10 @@ Decl *Parser::ParseFunctionDefinition(ParsingDeclarator &D, } else Actions.ActOnDefaultCtorInitializers(Res); + // Late attributes are parsed in the same scope as the function body. + if (LateParsedAttrs) + ParseLexedAttributeList(*LateParsedAttrs, Res, false, true); + return ParseFunctionStatementBody(Res, BodyScope); } diff --git a/test/Parser/attributes.c b/test/Parser/attributes.c index 36bd8071ac..347cb9c1bf 100644 --- a/test/Parser/attributes.c +++ b/test/Parser/attributes.c @@ -61,3 +61,38 @@ int aligned(int); int __attribute__((vec_type_hint(char, aligned(16) )) missing_rparen_1; // expected-error {{expected ')'}} int __attribute__((mode(x aligned(16) )) missing_rparen_2; // expected-error {{expected ')'}} int __attribute__((format(printf, 0 aligned(16) )) missing_rparen_3; // expected-error {{expected ')'}} + + + +int testFundef1(int *a) __attribute__((nonnull(1))) { // \ + // expected-warning {{GCC does not allow nonnull attribute in this position on a function definition}} + return *a; +} + +// noreturn is lifted to type qualifier +void testFundef2() __attribute__((noreturn)) { // \ + // expected-warning {{GCC does not allow noreturn attribute in this position on a function definition}} + testFundef2(); +} + +int testFundef3(int *a) __attribute__((nonnull(1), // \ + // expected-warning {{GCC does not allow nonnull attribute in this position on a function definition}} + pure)) { // \ + // expected-warning {{GCC does not allow pure attribute in this position on a function definition}} + return *a; +} + +int testFundef4(int *a) __attribute__((nonnull(1))) // \ + // expected-warning {{GCC does not allow nonnull attribute in this position on a function definition}} + __attribute((pure)) { // \ + // expected-warning {{GCC does not allow pure attribute in this position on a function definition}} + return *a; +} + +// GCC allows these +void testFundef5() __attribute__(()) { } + +__attribute__((pure)) int testFundef6(int a) { return a; } + + + diff --git a/test/SemaCXX/cxx0x-cursory-default-delete.cpp b/test/SemaCXX/cxx0x-cursory-default-delete.cpp index 17933c2f00..3290595616 100644 --- a/test/SemaCXX/cxx0x-cursory-default-delete.cpp +++ b/test/SemaCXX/cxx0x-cursory-default-delete.cpp @@ -67,3 +67,9 @@ struct except_spec_d_mismatch : except_spec_a, except_spec_b { struct except_spec_d_match : except_spec_a, except_spec_b { except_spec_d_match() throw(A, B) = default; }; + +// gcc-compatibility: allow attributes on default definitions +// (but not normal definitions) +struct S { S(); }; +S::S() __attribute((pure)) = default; + diff --git a/test/SemaCXX/warn-thread-safety-analysis.cpp b/test/SemaCXX/warn-thread-safety-analysis.cpp index 54e5795615..e0fde6b255 100644 --- a/test/SemaCXX/warn-thread-safety-analysis.cpp +++ b/test/SemaCXX/warn-thread-safety-analysis.cpp @@ -1936,3 +1936,118 @@ namespace GoingNative { } } + + + +namespace FunctionDefinitionTest { + +class Foo { +public: + void foo1(); + void foo2(); + void foo3(Foo *other); + + template + void fooT1(const T& dummy1); + + template + void fooT2(const T& dummy2) EXCLUSIVE_LOCKS_REQUIRED(mu_); + + Mutex mu_; + int a GUARDED_BY(mu_); +}; + +template +class FooT { +public: + void foo(); + + Mutex mu_; + T a GUARDED_BY(mu_); +}; + + +void Foo::foo1() NO_THREAD_SAFETY_ANALYSIS { + a = 1; +} + +void Foo::foo2() EXCLUSIVE_LOCKS_REQUIRED(mu_) { + a = 2; +} + +void Foo::foo3(Foo *other) EXCLUSIVE_LOCKS_REQUIRED(other->mu_) { + other->a = 3; +} + +template +void Foo::fooT1(const T& dummy1) EXCLUSIVE_LOCKS_REQUIRED(mu_) { + a = dummy1; +} + +/* TODO -- uncomment with template instantiation of attributes. +template +void Foo::fooT2(const T& dummy2) { + a = dummy2; +} +*/ + +void fooF1(Foo *f) EXCLUSIVE_LOCKS_REQUIRED(f->mu_) { + f->a = 1; +} + +void fooF2(Foo *f); +void fooF2(Foo *f) EXCLUSIVE_LOCKS_REQUIRED(f->mu_) { + f->a = 2; +} + +void fooF3(Foo *f) EXCLUSIVE_LOCKS_REQUIRED(f->mu_); +void fooF3(Foo *f) { + f->a = 3; +} + +template +void FooT::foo() EXCLUSIVE_LOCKS_REQUIRED(mu_) { + a = 0; +} + +void test() { + int dummy = 0; + Foo myFoo; + + myFoo.foo2(); // \ + // expected-warning {{calling function 'foo2' requires exclusive lock on 'mu_'}} + myFoo.foo3(&myFoo); // \ + // expected-warning {{calling function 'foo3' requires exclusive lock on 'mu_'}} + myFoo.fooT1(dummy); // \ + // expected-warning {{calling function 'fooT1' requires exclusive lock on 'mu_'}} + + // FIXME: uncomment with template instantiation of attributes patch + // myFoo.fooT2(dummy); // expected warning + + fooF1(&myFoo); // \ + // expected-warning {{calling function 'fooF1' requires exclusive lock on 'mu_'}} + fooF2(&myFoo); // \ + // expected-warning {{calling function 'fooF2' requires exclusive lock on 'mu_'}} + fooF3(&myFoo); // \ + // expected-warning {{calling function 'fooF3' requires exclusive lock on 'mu_'}} + + myFoo.mu_.Lock(); + myFoo.foo2(); + myFoo.foo3(&myFoo); + myFoo.fooT1(dummy); + + // FIXME: uncomment with template instantiation of attributes patch + // myFoo.fooT2(dummy); + + fooF1(&myFoo); + fooF2(&myFoo); + fooF3(&myFoo); + myFoo.mu_.Unlock(); + + FooT myFooT; + myFooT.foo(); // \ + // expected-warning {{calling function 'foo' requires exclusive lock on 'mu_'}} +} + +}; + diff --git a/test/SemaCXX/warn-thread-safety-parsing.cpp b/test/SemaCXX/warn-thread-safety-parsing.cpp index 6eacc761d5..c58ff9314e 100644 --- a/test/SemaCXX/warn-thread-safety-parsing.cpp +++ b/test/SemaCXX/warn-thread-safety-parsing.cpp @@ -1262,3 +1262,30 @@ class __attribute__((lockable)) EmptyArgListsTest { void unlock() __attribute__((unlock_function())) { } }; + +namespace FunctionDefinitionParseTest { +// Test parsing of attributes on function definitions. + +class Foo { +public: + Mu mu_; + void foo1(); + void foo2(Foo *f); +}; + +template +class Bar { +public: + Mu mu_; + void bar(); +}; + +void Foo::foo1() __attribute__((exclusive_locks_required(mu_))) { } +void Foo::foo2(Foo *f) __attribute__((exclusive_locks_required(f->mu_))) { } + +template +void Bar::bar() __attribute__((exclusive_locks_required(mu_))) { } + +void baz(Foo *f) __attribute__((exclusive_locks_required(f->mu_))) { } +}; +