From: Aaron Ballman Date: Mon, 31 Mar 2014 18:18:43 +0000 (+0000) Subject: Unify __declspec attribute argument parsing with the common attribute argument parsin... X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=ba9f3c5926ded09f7cd9268b1f3f1a6d40727e45;p=clang Unify __declspec attribute argument parsing with the common attribute argument parsing code. This removes a diagnostic that is no longer required (the semantic engine now properly handles attribute syntax so __declspec and __attribute__ spellings no longer get mismatched). This caused several testcases to need updating for a slightly different wording. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@205234 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/include/clang/Basic/DiagnosticParseKinds.td b/include/clang/Basic/DiagnosticParseKinds.td index f33160c3ef..40fab75f8a 100644 --- a/include/clang/Basic/DiagnosticParseKinds.td +++ b/include/clang/Basic/DiagnosticParseKinds.td @@ -535,8 +535,6 @@ def err_l_square_l_square_not_attribute : Error< "introducing an attribute">; def err_ms_declspec_type : Error< "__declspec attributes must be an identifier or string literal">; -def warn_ms_declspec_unknown : Warning< - "unknown __declspec attribute %0 ignored">, InGroup; def err_ms_property_no_getter_or_putter : Error< "property does not specify a getter or a putter">; def err_ms_property_unknown_accessor : Error< diff --git a/include/clang/Parse/Parser.h b/include/clang/Parse/Parser.h index 87a230f7e5..44f7cb1e05 100644 --- a/include/clang/Parse/Parser.h +++ b/include/clang/Parse/Parser.h @@ -2081,13 +2081,9 @@ private: void ParseMicrosoftAttributes(ParsedAttributes &attrs, SourceLocation *endLoc = 0); void ParseMicrosoftDeclSpec(ParsedAttributes &Attrs); - bool IsSimpleMicrosoftDeclSpec(IdentifierInfo *Ident); - void ParseComplexMicrosoftDeclSpec(IdentifierInfo *Ident, - SourceLocation Loc, - ParsedAttributes &Attrs); - void ParseMicrosoftDeclSpecWithSingleArg(IdentifierInfo *AttrName, - SourceLocation AttrNameLoc, - ParsedAttributes &Attrs); + bool ParseMicrosoftDeclSpecArgs(IdentifierInfo *AttrName, + SourceLocation AttrNameLoc, + ParsedAttributes &Attrs); void ParseMicrosoftTypeAttributes(ParsedAttributes &attrs); void ParseMicrosoftInheritanceClassAttributes(ParsedAttributes &attrs); void ParseBorlandTypeAttributes(ParsedAttributes &attrs); diff --git a/lib/Parse/ParseDecl.cpp b/lib/Parse/ParseDecl.cpp index 540bc8ef60..331c5e42fd 100644 --- a/lib/Parse/ParseDecl.cpp +++ b/lib/Parse/ParseDecl.cpp @@ -16,7 +16,9 @@ #include "clang/AST/ASTContext.h" #include "clang/AST/DeclTemplate.h" #include "clang/Basic/AddressSpaces.h" +#include "clang/Basic/Attributes.h" #include "clang/Basic/CharInfo.h" +#include "clang/Basic/TargetInfo.h" #include "clang/Parse/ParseDiagnostic.h" #include "clang/Sema/Lookup.h" #include "clang/Sema/ParsedTemplate.h" @@ -377,91 +379,34 @@ void Parser::ParseGNUAttributeArgs(IdentifierInfo *AttrName, ScopeLoc, Syntax); } -/// \brief Parses a single argument for a declspec, including the -/// surrounding parens. -void Parser::ParseMicrosoftDeclSpecWithSingleArg(IdentifierInfo *AttrName, - SourceLocation AttrNameLoc, - ParsedAttributes &Attrs) -{ - BalancedDelimiterTracker T(*this, tok::l_paren); - if (T.expectAndConsume(diag::err_expected_lparen_after, - AttrName->getNameStart(), tok::r_paren)) - return; - - ExprResult ArgExpr(ParseConstantExpression()); - if (ArgExpr.isInvalid()) { - T.skipToEnd(); - return; +bool Parser::ParseMicrosoftDeclSpecArgs(IdentifierInfo *AttrName, + SourceLocation AttrNameLoc, + ParsedAttributes &Attrs) { + // If the attribute isn't known, we will not attempt to parse any + // arguments. + if (!hasAttribute(AttrSyntax::Declspec, nullptr, AttrName, + getTargetInfo().getTriple(), getLangOpts())) { + // Eat the left paren, then skip to the ending right paren. + ConsumeParen(); + SkipUntil(tok::r_paren); + return false; } - ArgsUnion ExprList = ArgExpr.take(); - Attrs.addNew(AttrName, AttrNameLoc, 0, AttrNameLoc, &ExprList, 1, - AttributeList::AS_Declspec); - - T.consumeClose(); -} - -/// \brief Determines whether a declspec is a "simple" one requiring no -/// arguments. -bool Parser::IsSimpleMicrosoftDeclSpec(IdentifierInfo *Ident) { - return llvm::StringSwitch(Ident->getName()) - .Case("dllimport", true) - .Case("dllexport", true) - .Case("noreturn", true) - .Case("nothrow", true) - .Case("noinline", true) - .Case("naked", true) - .Case("appdomain", true) - .Case("process", true) - .Case("jitintrinsic", true) - .Case("noalias", true) - .Case("restrict", true) - .Case("novtable", true) - .Case("selectany", true) - .Case("thread", true) - .Case("safebuffers", true ) - .Default(false); -} -/// \brief Attempts to parse a declspec which is not simple (one that takes -/// parameters). Will return false if we properly handled the declspec, or -/// true if it is an unknown declspec. -void Parser::ParseComplexMicrosoftDeclSpec(IdentifierInfo *Ident, - SourceLocation Loc, - ParsedAttributes &Attrs) { - // Try to handle the easy case first -- these declspecs all take a single - // parameter as their argument. - if (llvm::StringSwitch(Ident->getName()) - .Case("uuid", true) - .Case("align", true) - .Case("allocate", true) - .Default(false)) { - ParseMicrosoftDeclSpecWithSingleArg(Ident, Loc, Attrs); - } else if (Ident->getName() == "deprecated") { - // The deprecated declspec has an optional single argument, so we will - // check for a l-paren to decide whether we should parse an argument or - // not. - if (Tok.getKind() == tok::l_paren) - ParseMicrosoftDeclSpecWithSingleArg(Ident, Loc, Attrs); - else - Attrs.addNew(Ident, Loc, 0, Loc, 0, 0, AttributeList::AS_Declspec); - } else if (Ident->getName() == "property") { + if (AttrName->getName() == "property") { // The property declspec is more complex in that it can take one or two // assignment expressions as a parameter, but the lhs of the assignment // must be named get or put. - if (Tok.isNot(tok::l_paren)) { - Diag(Tok.getLocation(), diag::err_expected_lparen_after) - << Ident->getNameStart(); - return; - } + BalancedDelimiterTracker T(*this, tok::l_paren); T.expectAndConsume(diag::err_expected_lparen_after, - Ident->getNameStart(), tok::r_paren); + AttrName->getNameStart(), tok::r_paren); enum AccessorKind { AK_Invalid = -1, - AK_Put = 0, AK_Get = 1 // indices into AccessorNames + AK_Put = 0, + AK_Get = 1 // indices into AccessorNames }; - IdentifierInfo *AccessorNames[] = { 0, 0 }; + IdentifierInfo *AccessorNames[] = {0, 0}; bool HasInvalidAccessor = false; // Parse the accessor specifications. @@ -471,7 +416,7 @@ void Parser::ParseComplexMicrosoftDeclSpec(IdentifierInfo *Ident, // If the user wrote a completely empty list, use a special diagnostic. if (Tok.is(tok::r_paren) && !HasInvalidAccessor && AccessorNames[AK_Put] == 0 && AccessorNames[AK_Get] == 0) { - Diag(Loc, diag::err_ms_property_no_getter_or_putter); + Diag(AttrNameLoc, diag::err_ms_property_no_getter_or_putter); break; } @@ -487,28 +432,29 @@ void Parser::ParseComplexMicrosoftDeclSpec(IdentifierInfo *Ident, } else if (KindStr == "put") { Kind = AK_Put; - // Recover from the common mistake of using 'set' instead of 'put'. + // Recover from the common mistake of using 'set' instead of 'put'. } else if (KindStr == "set") { Diag(KindLoc, diag::err_ms_property_has_set_accessor) - << FixItHint::CreateReplacement(KindLoc, "put"); + << FixItHint::CreateReplacement(KindLoc, "put"); Kind = AK_Put; - // Handle the mistake of forgetting the accessor kind by skipping - // this accessor. + // Handle the mistake of forgetting the accessor kind by skipping + // this accessor. } else if (NextToken().is(tok::comma) || NextToken().is(tok::r_paren)) { Diag(KindLoc, diag::err_ms_property_missing_accessor_kind); ConsumeToken(); HasInvalidAccessor = true; goto next_property_accessor; - // Otherwise, complain about the unknown accessor kind. + // Otherwise, complain about the unknown accessor kind. } else { Diag(KindLoc, diag::err_ms_property_unknown_accessor); HasInvalidAccessor = true; Kind = AK_Invalid; // Try to keep parsing unless it doesn't look like an accessor spec. - if (!NextToken().is(tok::equal)) break; + if (!NextToken().is(tok::equal)) + break; } // Consume the identifier. @@ -517,7 +463,7 @@ void Parser::ParseComplexMicrosoftDeclSpec(IdentifierInfo *Ident, // Consume the '='. if (!TryConsumeToken(tok::equal)) { Diag(Tok.getLocation(), diag::err_ms_property_expected_equal) - << KindStr; + << KindStr; break; } @@ -551,27 +497,17 @@ void Parser::ParseComplexMicrosoftDeclSpec(IdentifierInfo *Ident, } // Only add the property attribute if it was well-formed. - if (!HasInvalidAccessor) { - Attrs.addNewPropertyAttr(Ident, Loc, 0, SourceLocation(), + if (!HasInvalidAccessor) + Attrs.addNewPropertyAttr(AttrName, AttrNameLoc, 0, SourceLocation(), AccessorNames[AK_Get], AccessorNames[AK_Put], AttributeList::AS_Declspec); - } T.skipToEnd(); - } else { - // We don't recognize this as a valid declspec, but instead of creating the - // attribute and allowing sema to warn about it, we will warn here instead. - // This is because some attributes have multiple spellings, but we need to - // disallow that for declspecs (such as align vs aligned). If we made the - // attribute, we'd have to split the valid declspec spelling logic into - // both locations. - Diag(Loc, diag::warn_ms_declspec_unknown) << Ident; - - // If there's an open paren, we should eat the open and close parens under - // the assumption that this unknown declspec has parameters. - BalancedDelimiterTracker T(*this, tok::l_paren); - if (!T.consumeOpen()) - T.skipToEnd(); + return !HasInvalidAccessor; } + + ParseAttributeArgsCommon(AttrName, AttrNameLoc, Attrs, nullptr, nullptr, + SourceLocation(), AttributeList::AS_Declspec); + return true; } /// [MS] decl-specifier: @@ -591,7 +527,11 @@ void Parser::ParseMicrosoftDeclSpec(ParsedAttributes &Attrs) { // An empty declspec is perfectly legal and should not warn. Additionally, // you can specify multiple attributes per declspec. - while (Tok.getKind() != tok::r_paren) { + while (Tok.isNot(tok::r_paren)) { + // Attribute not present. + if (TryConsumeToken(tok::comma)) + continue; + // We expect either a well-known identifier or a generic string. Anything // else is a malformed declspec. bool IsString = Tok.getKind() == tok::string_literal ? true : false; @@ -619,17 +559,19 @@ void Parser::ParseMicrosoftDeclSpec(ParsedAttributes &Attrs) { AttrNameLoc = ConsumeToken(); } - if (IsString || IsSimpleMicrosoftDeclSpec(AttrName)) - // If we have a generic string, we will allow it because there is no - // documented list of allowable string declspecs, but we know they exist - // (for instance, SAL declspecs in older versions of MSVC). - // - // Alternatively, if the identifier is a simple one, then it requires no - // arguments and can be turned into an attribute directly. + bool AttrHandled = false; + + // Parse attribute arguments. + if (Tok.is(tok::l_paren)) + AttrHandled = ParseMicrosoftDeclSpecArgs(AttrName, AttrNameLoc, Attrs); + else if (AttrName->getName() == "property") + // The property attribute must have an argument list. + Diag(Tok.getLocation(), diag::err_expected_lparen_after) + << AttrName->getName(); + + if (!AttrHandled) Attrs.addNew(AttrName, AttrNameLoc, 0, AttrNameLoc, 0, 0, AttributeList::AS_Declspec); - else - ParseComplexMicrosoftDeclSpec(AttrName, AttrNameLoc, Attrs); } T.consumeClose(); } diff --git a/test/Parser/MicrosoftExtensions.c b/test/Parser/MicrosoftExtensions.c index f61f7b45f1..b9f531fb1e 100644 --- a/test/Parser/MicrosoftExtensions.c +++ b/test/Parser/MicrosoftExtensions.c @@ -98,7 +98,7 @@ void ms_intrinsics(int a) __debugbreak(); } -struct __declspec(frobble) S1 {}; /* expected-warning {{unknown __declspec attribute 'frobble' ignored}} */ +struct __declspec(frobble) S1 {}; /* expected-warning {{__declspec attribute 'frobble' is not supported}} */ struct __declspec(12) S2 {}; /* expected-error {{__declspec attributes must be an identifier or string literal}} */ struct __declspec("testing") S3 {}; /* expected-warning {{__declspec attribute '"testing"' is not supported}} */ @@ -106,8 +106,8 @@ struct __declspec("testing") S3 {}; /* expected-warning {{__declspec attribute ' struct __declspec(align(8) deprecated) S4 {}; /* But multiple declspecs must still be legal */ -struct __declspec(deprecated frobble "testing") S5 {}; /* expected-warning {{unknown __declspec attribute 'frobble' ignored}} expected-warning {{__declspec attribute '"testing"' is not supported}} */ -struct __declspec(unknown(12) deprecated) S6 {}; /* expected-warning {{unknown __declspec attribute 'unknown' ignored}}*/ +struct __declspec(deprecated frobble "testing") S5 {}; /* expected-warning {{__declspec attribute 'frobble' is not supported}} expected-warning {{__declspec attribute '"testing"' is not supported}} */ +struct __declspec(unknown(12) deprecated) S6 {}; /* expected-warning {{__declspec attribute 'unknown' is not supported}}*/ struct S7 { int foo() { return 12; } diff --git a/test/Sema/MicrosoftCompatibility.c b/test/Sema/MicrosoftCompatibility.c index cc18583f6f..a193b26b42 100644 --- a/test/Sema/MicrosoftCompatibility.c +++ b/test/Sema/MicrosoftCompatibility.c @@ -16,8 +16,8 @@ __declspec(noreturn) void f6( void ) { } __declspec(align(32768)) struct S1 { int a; } s; /* expected-error {{requested alignment must be 8192 bytes or smaller}} */ -struct __declspec(aligned) S2 {}; /* expected-warning {{unknown __declspec attribute 'aligned' ignored}} */ +struct __declspec(aligned) S2 {}; /* expected-warning {{__declspec attribute 'aligned' is not supported}} */ struct __declspec(appdomain) S3 {}; /* expected-warning {{__declspec attribute 'appdomain' is not supported}} */ -__declspec(__noreturn__) void f7(void); /* expected-warning {{unknown __declspec attribute '__noreturn__' ignored}} */ +__declspec(__noreturn__) void f7(void); /* expected-warning {{__declspec attribute '__noreturn__' is not supported}} */ diff --git a/test/Sema/pragma-ms_struct.c b/test/Sema/pragma-ms_struct.c index 8cce4d7981..e2c5ff1f48 100644 --- a/test/Sema/pragma-ms_struct.c +++ b/test/Sema/pragma-ms_struct.c @@ -59,5 +59,5 @@ void *pv2; static int arr[sizeof(PackOddity) == 40 ? 1 : -1]; -__declspec(ms_struct) struct bad { // expected-warning {{unknown __declspec attribute 'ms_struct' ignored}} +struct __declspec(ms_struct) bad { // expected-warning {{__declspec attribute 'ms_struct' is not supported}} };