Index: lib/Lex/PPMacroExpansion.cpp =================================================================== --- lib/Lex/PPMacroExpansion.cpp +++ lib/Lex/PPMacroExpansion.cpp @@ -1047,9 +1047,8 @@ /// HasFeature - Return true if we recognize and implement the feature /// specified by the identifier as a standard language feature. -static bool HasFeature(const Preprocessor &PP, const IdentifierInfo *II) { +static bool HasFeature(const Preprocessor &PP, StringRef Feature) { const LangOptions &LangOpts = PP.getLangOpts(); - StringRef Feature = II->getName(); // Normalize the feature name, __foo__ becomes foo. if (Feature.startswith("__") && Feature.endswith("__") && Feature.size() >= 4) @@ -1218,8 +1217,8 @@ /// HasExtension - Return true if we recognize and implement the feature /// specified by the identifier, either as an extension or a standard language /// feature. -static bool HasExtension(const Preprocessor &PP, const IdentifierInfo *II) { - if (HasFeature(PP, II)) +static bool HasExtension(const Preprocessor &PP, StringRef Extension) { + if (HasFeature(PP, Extension)) return true; // If the use of an extension results in an error diagnostic, extensions are @@ -1229,7 +1228,6 @@ return false; const LangOptions &LangOpts = PP.getLangOpts(); - StringRef Extension = II->getName(); // Normalize the extension name, __foo__ becomes foo. if (Extension.startswith("__") && Extension.endswith("__") && @@ -1413,47 +1411,115 @@ return EvaluateHasIncludeCommon(Tok, II, PP, Lookup, LookupFromFile); } -/// \brief Process __building_module(identifier) expression. -/// \returns true if we are building the named module, false otherwise. -static bool EvaluateBuildingModule(Token &Tok, - IdentifierInfo *II, Preprocessor &PP) { - // Get '('. - PP.LexNonComment(Tok); - - // Ensure we have a '('. +/// \brief Process single-argument builtin feature-like macros that return +/// integer values. +static void EvaluateFeatureLikeBuiltinMacro(llvm::raw_svector_ostream& OS, + Token &Tok, IdentifierInfo *II, + Preprocessor &PP, + int(*Op)(Token &Tok, + Preprocessor &PP, + bool &HasLexedNextTok)) { + // Parse the initial '('. + PP.LexUnexpandedNonComment(Tok); if (Tok.isNot(tok::l_paren)) { PP.Diag(Tok.getLocation(), diag::err_pp_expected_after) << II << tok::l_paren; - return false; + + // Provide a dummy '0' value on output stream to elide further errors. + if (!Tok.isOneOf(tok::eof, tok::eod)) { + OS << 0; + Tok.setKind(tok::numeric_constant); + } + return; } - // Save '(' location for possible missing ')' message. - SourceLocation LParenLoc = Tok.getLocation(); + llvm::Optional Result; - // Get the module name. - PP.LexNonComment(Tok); + // It is possible that the macro invocation has a multiple-nested argument, + // for example: __has_feature((((X)))). This should be counted as valid and + // parsed correctly. + SmallVector ParenStack; + ParenStack.push_back(Tok.getLocation()); + + bool SuppressDiagnostic = false; + while (true) { + // Parse next non-comment token. + PP.LexUnexpandedNonComment(Tok); + +already_lexed: + switch (Tok.getKind()) { + case tok::eof: + case tok::eod: + // Don't provide even a dummy value if the eod or eof marker is + // reached. Simply provide a diagnostic. + PP.Diag(Tok.getLocation(), diag::err_unterm_macro_invoc); + return; - // Ensure that we have an identifier. - if (Tok.isNot(tok::identifier)) { - PP.Diag(Tok.getLocation(), diag::err_expected_id_building_module); - return false; - } + case tok::comma: + if (!SuppressDiagnostic) { + PP.Diag(Tok.getLocation(), diag::err_too_many_args_in_macro_invoc); + SuppressDiagnostic = true; + } + continue; - bool Result - = Tok.getIdentifierInfo()->getName() == PP.getLangOpts().CurrentModule; + case tok::l_paren: + ParenStack.push_back(Tok.getLocation()); + if (!Result.hasValue()) + continue; + break; - // Get ')'. - PP.LexNonComment(Tok); + case tok::r_paren: + ParenStack.pop_back(); + if (ParenStack.size() > 0) + continue; + + // The last ')' has been reached; return the value if one found or + // a diagnostic and a dummy value. + if (Result.hasValue()) + OS << Result.getValue(); + else { + OS << 0; + if (!SuppressDiagnostic) + PP.Diag(Tok.getLocation(), diag::err_too_few_args_in_macro_invoc); + } + Tok.setKind(tok::numeric_constant); + return; - // Ensure we have a trailing ). - if (Tok.isNot(tok::r_paren)) { - PP.Diag(Tok.getLocation(), diag::err_pp_expected_after) << II - << tok::r_paren; - PP.Diag(LParenLoc, diag::note_matching) << tok::l_paren; - return false; + default: { + // Parse the macro argument, if one not found so far. + if (Result.hasValue()) + break; + + bool HasLexedNextToken = false; + Result = Op(Tok, PP, HasLexedNextToken); + if (HasLexedNextToken) + goto already_lexed; + continue; + } + } + + // The macro argument has already been parsed; ')' is all that is accepted + // now. Keep parsing until the last ')' is reached. + if (!SuppressDiagnostic) { + PP.Diag(Tok.getLocation(), diag::err_pp_expected_after) << II + << tok::r_paren; + PP.Diag(ParenStack.back(), diag::note_matching) << tok::l_paren; + SuppressDiagnostic = true; + } } +} + +/// \brief Helper function to return the IdentifierInfo structure of a Token +/// or generate a diagnostic if none available. +static IdentifierInfo *ExpectFeatureIdentifierInfo(Token &Tok, + Preprocessor &PP, + signed DiagID) { + IdentifierInfo *II; + if (!Tok.isAnnotation() && (II = Tok.getIdentifierInfo())) + return II; - return Result; + PP.Diag(Tok.getLocation(), DiagID); + return nullptr; } /// ExpandBuiltinMacro - If an identifier token is read that is to be expanded @@ -1589,84 +1655,81 @@ // __COUNTER__ expands to a simple numeric value. OS << CounterValue++; Tok.setKind(tok::numeric_constant); - } else if (II == Ident__has_feature || - II == Ident__has_extension || - II == Ident__has_builtin || - II == Ident__is_identifier || - II == Ident__has_attribute || - II == Ident__has_declspec || - II == Ident__has_cpp_attribute) { - // The argument to these builtins should be a parenthesized identifier. - SourceLocation StartLoc = Tok.getLocation(); - - bool IsValid = false; - IdentifierInfo *FeatureII = nullptr; - IdentifierInfo *ScopeII = nullptr; - - // Read the '('. - LexUnexpandedToken(Tok); - if (Tok.is(tok::l_paren)) { - // Read the identifier - LexUnexpandedToken(Tok); - if ((FeatureII = Tok.getIdentifierInfo())) { - // If we're checking __has_cpp_attribute, it is possible to receive a - // scope token. Read the "::", if it's available. - LexUnexpandedToken(Tok); - bool IsScopeValid = true; - if (II == Ident__has_cpp_attribute && Tok.is(tok::coloncolon)) { - LexUnexpandedToken(Tok); - // The first thing we read was not the feature, it was the scope. - ScopeII = FeatureII; - if ((FeatureII = Tok.getIdentifierInfo())) - LexUnexpandedToken(Tok); - else - IsScopeValid = false; + } else if (II == Ident__has_feature) { + EvaluateFeatureLikeBuiltinMacro(OS, Tok, II, *this, + [](Token &Tok, Preprocessor &PP, bool &HasLexedNextToken) -> int { + IdentifierInfo *II = ExpectFeatureIdentifierInfo(Tok, PP, + diag::err_feature_check_malformed); + return II && HasFeature(PP, II->getName()); + }); + } else if (II == Ident__has_extension) { + EvaluateFeatureLikeBuiltinMacro(OS, Tok, II, *this, + [](Token &Tok, Preprocessor &PP, bool &HasLexedNextToken) -> int { + IdentifierInfo *II = ExpectFeatureIdentifierInfo(Tok, PP, + diag::err_feature_check_malformed); + return II && HasExtension(PP, II->getName()); + }); + } else if (II == Ident__has_builtin) { + EvaluateFeatureLikeBuiltinMacro(OS, Tok, II, *this, + [](Token &Tok, Preprocessor &PP, bool &HasLexedNextToken) -> int { + IdentifierInfo *II = ExpectFeatureIdentifierInfo(Tok, PP, + diag::err_feature_check_malformed); + if (!II) + return false; + else if (II->getBuiltinID() != 0) + return true; + else { + const LangOptions &LangOpts = PP.getLangOpts(); + return llvm::StringSwitch(II->getName()) + .Case("__make_integer_seq", LangOpts.CPlusPlus) + .Default(false); + } + }); + } else if (II == Ident__is_identifier) { + EvaluateFeatureLikeBuiltinMacro(OS, Tok, II, *this, + [](Token &Tok, Preprocessor &PP, bool &HasLexedNextToken) -> int { + return Tok.is(tok::identifier); + }); + } else if (II == Ident__has_attribute) { + EvaluateFeatureLikeBuiltinMacro(OS, Tok, II, *this, + [](Token &Tok, Preprocessor &PP, bool &HasLexedNextToken) -> int { + IdentifierInfo *II = ExpectFeatureIdentifierInfo(Tok, PP, + diag::err_feature_check_malformed); + return II ? hasAttribute(AttrSyntax::GNU, nullptr, II, + PP.getTargetInfo(), PP.getLangOpts()) : 0; + }); + } else if (II == Ident__has_declspec) { + EvaluateFeatureLikeBuiltinMacro(OS, Tok, II, *this, + [](Token &Tok, Preprocessor &PP, bool &HasLexedNextToken) -> int { + IdentifierInfo *II = ExpectFeatureIdentifierInfo(Tok, PP, + diag::err_feature_check_malformed); + return II ? hasAttribute(AttrSyntax::Declspec, nullptr, II, + PP.getTargetInfo(), PP.getLangOpts()) : 0; + }); + } else if (II == Ident__has_cpp_attribute) { + EvaluateFeatureLikeBuiltinMacro(OS, Tok, II, *this, + [](Token &Tok, Preprocessor &PP, bool &HasLexedNextToken) -> int { + IdentifierInfo *ScopeII = nullptr; + IdentifierInfo *II = ExpectFeatureIdentifierInfo(Tok, PP, + diag::err_feature_check_malformed); + if (!II) + return false; + + // It is possible to receive a scope token. Read the "::", if it is + // available, and the subsequent identifier. + PP.LexUnexpandedToken(Tok); + if (Tok.isNot(tok::coloncolon)) + HasLexedNextToken = true; + else { + ScopeII = II; + PP.LexUnexpandedToken(Tok); + II = ExpectFeatureIdentifierInfo(Tok, PP, + diag::err_feature_check_malformed); } - // Read the closing paren. - if (IsScopeValid && Tok.is(tok::r_paren)) - IsValid = true; - } - // Eat tokens until ')'. - while (Tok.isNot(tok::r_paren) && Tok.isNot(tok::eod) && - Tok.isNot(tok::eof)) - LexUnexpandedToken(Tok); - } - - int Value = 0; - if (!IsValid) - Diag(StartLoc, diag::err_feature_check_malformed); - else if (II == Ident__is_identifier) - Value = FeatureII->getTokenID() == tok::identifier; - else if (II == Ident__has_builtin) { - // Check for a builtin is trivial. - if (FeatureII->getBuiltinID() != 0) { - Value = true; - } else { - StringRef Feature = FeatureII->getName(); - Value = llvm::StringSwitch(Feature) - .Case("__make_integer_seq", getLangOpts().CPlusPlus) - .Default(false); - } - } else if (II == Ident__has_attribute) - Value = hasAttribute(AttrSyntax::GNU, nullptr, FeatureII, - getTargetInfo(), getLangOpts()); - else if (II == Ident__has_cpp_attribute) - Value = hasAttribute(AttrSyntax::CXX, ScopeII, FeatureII, - getTargetInfo(), getLangOpts()); - else if (II == Ident__has_declspec) - Value = hasAttribute(AttrSyntax::Declspec, nullptr, FeatureII, - getTargetInfo(), getLangOpts()); - else if (II == Ident__has_extension) - Value = HasExtension(*this, FeatureII); - else { - assert(II == Ident__has_feature && "Must be feature check"); - Value = HasFeature(*this, FeatureII); - } - if (!IsValid) - return; - OS << Value; - Tok.setKind(tok::numeric_constant); + return II ? hasAttribute(AttrSyntax::CXX, ScopeII, II, + PP.getTargetInfo(), PP.getLangOpts()) : 0; + }); } else if (II == Ident__has_include || II == Ident__has_include_next) { // The argument to these two builtins should be a parenthesized @@ -1684,64 +1747,43 @@ Tok.setKind(tok::numeric_constant); } else if (II == Ident__has_warning) { // The argument should be a parenthesized string literal. - // The argument to these builtins should be a parenthesized identifier. - SourceLocation StartLoc = Tok.getLocation(); - bool IsValid = false; - bool Value = false; - // Read the '('. - LexUnexpandedToken(Tok); - do { - if (Tok.isNot(tok::l_paren)) { - Diag(StartLoc, diag::err_warning_check_malformed); - break; - } - - LexUnexpandedToken(Tok); - std::string WarningName; - SourceLocation StrStartLoc = Tok.getLocation(); - if (!FinishLexStringLiteral(Tok, WarningName, "'__has_warning'", - /*MacroExpansion=*/false)) { - // Eat tokens until ')'. - while (Tok.isNot(tok::r_paren) && Tok.isNot(tok::eod) && - Tok.isNot(tok::eof)) - LexUnexpandedToken(Tok); - break; - } - - // Is the end a ')'? - if (!(IsValid = Tok.is(tok::r_paren))) { - Diag(StartLoc, diag::err_warning_check_malformed); - break; - } - - // FIXME: Should we accept "-R..." flags here, or should that be handled - // by a separate __has_remark? - if (WarningName.size() < 3 || WarningName[0] != '-' || - WarningName[1] != 'W') { - Diag(StrStartLoc, diag::warn_has_warning_invalid_option); - break; - } - - // Finally, check if the warning flags maps to a diagnostic group. - // We construct a SmallVector here to talk to getDiagnosticIDs(). - // Although we don't use the result, this isn't a hot path, and not - // worth special casing. - SmallVector Diags; - Value = !getDiagnostics().getDiagnosticIDs()-> - getDiagnosticsInGroup(diag::Flavor::WarningOrError, - WarningName.substr(2), Diags); - } while (false); + EvaluateFeatureLikeBuiltinMacro(OS, Tok, II, *this, + [](Token &Tok, Preprocessor &PP, bool &HasLexedNextToken) -> int { + std::string WarningName; + SourceLocation StrStartLoc = Tok.getLocation(); + + HasLexedNextToken = Tok.is(tok::string_literal); + if (!PP.FinishLexStringLiteral(Tok, WarningName, "'__has_warning'", + /*MacroExpansion=*/false)) + return false; + + // FIXME: Should we accept "-R..." flags here, or should that be + // handled by a separate __has_remark? + if (WarningName.size() < 3 || WarningName[0] != '-' || + WarningName[1] != 'W') { + PP.Diag(StrStartLoc, diag::warn_has_warning_invalid_option); + return false; + } - if (!IsValid) - return; - OS << (int)Value; - Tok.setKind(tok::numeric_constant); + // Finally, check if the warning flags maps to a diagnostic group. + // We construct a SmallVector here to talk to getDiagnosticIDs(). + // Although we don't use the result, this isn't a hot path, and not + // worth special casing. + SmallVector Diags; + return !PP.getDiagnostics().getDiagnosticIDs()-> + getDiagnosticsInGroup(diag::Flavor::WarningOrError, + WarningName.substr(2), Diags); + }); } else if (II == Ident__building_module) { // The argument to this builtin should be an identifier. The // builtin evaluates to 1 when that identifier names the module we are // currently building. - OS << (int)EvaluateBuildingModule(Tok, II, *this); - Tok.setKind(tok::numeric_constant); + EvaluateFeatureLikeBuiltinMacro(OS, Tok, II, *this, + [](Token &Tok, Preprocessor &PP, bool &HasLexedNextToken) -> int { + IdentifierInfo *II = ExpectFeatureIdentifierInfo(Tok, PP, + diag::err_expected_id_building_module); + return II && (II->getName() == PP.getLangOpts().CurrentModule); + }); } else if (II == Ident__MODULE__) { // The current module as an identifier. OS << getLangOpts().CurrentModule; Index: test/Preprocessor/feature_tests.c =================================================================== --- test/Preprocessor/feature_tests.c +++ test/Preprocessor/feature_tests.c @@ -55,8 +55,41 @@ #endif #ifdef VERIFY -// expected-error@+2 {{builtin feature check macro requires a parenthesized identifier}} -// expected-error@+1 {{expected value in expression}} +// expected-error@+1 {{builtin feature check macro requires a parenthesized identifier}} #if __has_feature('x') #endif + +// The following are not identifiers: +_Static_assert(!__is_identifier("string"), "oops"); +_Static_assert(!__is_identifier('c'), "oops"); +_Static_assert(!__is_identifier(123), "oops"); + +// The following are: +_Static_assert(__is_identifier(abc /* comment */), "oops"); +_Static_assert(__is_identifier /* comment */ (((abc))), "oops"); + +// expected-error@+1 {{too many arguments}} +#if __is_identifier(,()) +#endif + +// expected-error@+1 {{missing ')' after '__is_identifier'}} +#if __is_identifier(x x) // expected-note {{to match this '('}} +#endif + +// expected-error@+1 {{missing ')' after '__is_identifier'}} +#if __is_identifier(x()) // expected-note {{to match this '('}} +#endif + +// expected-error@+1 {{missing ')' after '__is_identifier'}} +#if __is_identifier(x.) // expected-note {{to match this '('}} +#endif + +// expected-error@+1 {{missing '('}} expected-error@+1 {{expected value}} +#if __is_identifier +#endif + +// expected-error@+1 {{unterminated}} expected-error@+1 {{expected value}} +#if __is_identifier( +#endif + #endif Index: test/Preprocessor/invalid-__has_warning1.c =================================================================== --- test/Preprocessor/invalid-__has_warning1.c +++ test/Preprocessor/invalid-__has_warning1.c @@ -1,5 +1,5 @@ // RUN: %clang_cc1 -verify %s // These must be the last lines in this test. -// expected-error@+1{{expected string literal}} expected-error@+1 2{{expected}} +// expected-error@+1{{unterminated}} expected-error@+1 2{{expected}} int i = __has_warning( Index: test/Preprocessor/invalid-__has_warning2.c =================================================================== --- test/Preprocessor/invalid-__has_warning2.c +++ test/Preprocessor/invalid-__has_warning2.c @@ -1,5 +1,5 @@ // RUN: %clang_cc1 -verify %s // These must be the last lines in this test. -// expected-error@+1{{expected string literal}} expected-error@+1{{expected}} +// expected-error@+1{{too few arguments}} int i = __has_warning(); Index: test/Preprocessor/warning_tests.c =================================================================== --- test/Preprocessor/warning_tests.c +++ test/Preprocessor/warning_tests.c @@ -12,7 +12,7 @@ #endif // expected-error@+2 {{expected string literal in '__has_warning'}} -// expected-error@+1 {{expected value in expression}} +// expected-error@+1 {{missing ')'}} expected-note@+1 {{match}} #if __has_warning(-Wfoo) #endif @@ -22,8 +22,7 @@ #warning Not a valid warning flag #endif -// expected-error@+2 {{builtin warning check macro requires a parenthesized string}} -// expected-error@+1 {{invalid token}} +// expected-error@+1 {{missing '(' after '__has_warning'}} #if __has_warning "not valid" #endif @@ -33,7 +32,7 @@ #define MY_ALIAS "-Wparentheses" -// expected-error@+1 2{{expected}} +// expected-error@+1 {{expected}} #if __has_warning(MY_ALIAS) #error Alias expansion not allowed #endif