AndyG updated this revision to Diff 52513.
AndyG added a comment.
Ok, I've removed support for nested parentheses. Can this go through now?
Thanks.
http://reviews.llvm.org/D17149
Files:
include/clang/Basic/DiagnosticLexKinds.td
lib/Lex/PPMacroExpansion.cpp
test/Preprocessor/feature_tests.c
test/Preprocessor/invalid-__has_warning1.c
test/Preprocessor/invalid-__has_warning2.c
test/Preprocessor/warning_tests.c
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
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/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/feature_tests.c
===================================================================
--- test/Preprocessor/feature_tests.c
+++ test/Preprocessor/feature_tests.c
@@ -55,8 +55,50 @@
#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");
+_Static_assert(!__is_identifier(int), "oops");
+
+// The following are:
+_Static_assert(__is_identifier(abc /* comment */), "oops");
+_Static_assert(__is_identifier /* comment */ (xyz), "oops");
+
+// expected-error@+1 {{too few arguments}}
+#if __is_identifier()
+#endif
+
+// expected-error@+1 {{too many arguments}}
+#if __is_identifier(,())
+#endif
+
+// expected-error@+1 {{missing ')' after 'abc'}}
+#if __is_identifier(abc xyz) // expected-note {{to match this '('}}
+#endif
+
+// expected-error@+1 {{missing ')' after 'abc'}}
+#if __is_identifier(abc()) // expected-note {{to match this '('}}
+#endif
+
+// expected-error@+1 {{missing ')' after 'abc'}}
+#if __is_identifier(abc.) // expected-note {{to match this '('}}
+#endif
+
+// expected-error@+1 {{'(' not expected after '('}}
+#if __is_identifier((abc))
+#endif
+
+// expected-error@+1 {{missing '(' after '__is_identifier'}} expected-error@+1 {{expected value}}
+#if __is_identifier
+#endif
+
+// expected-error@+1 {{unterminated}} expected-error@+1 {{expected value}}
+#if __is_identifier(
+#endif
+
#endif
Index: lib/Lex/PPMacroExpansion.cpp
===================================================================
--- lib/Lex/PPMacroExpansion.cpp
+++ lib/Lex/PPMacroExpansion.cpp
@@ -1053,9 +1053,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)
@@ -1229,8 +1228,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
@@ -1240,7 +1239,6 @@
return false;
const LangOptions &LangOpts = PP.getLangOpts();
- StringRef Extension = II->getName();
// Normalize the extension name, __foo__ becomes foo.
if (Extension.startswith("__") && Extension.endswith("__") &&
@@ -1424,48 +1422,126 @@
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.
+ unsigned ParenDepth = 1;
SourceLocation LParenLoc = Tok.getLocation();
+ llvm::Optional<int> Result;
- // Get the module name.
- PP.LexNonComment(Tok);
+ bool SuppressDiagnostic = false;
+ while (true) {
+ // Keep last token for diagnostics.
+ Token LastTok = Tok;
+
+ // Parse next non-comment, non-annotation token.
+ do PP.LexUnexpandedNonComment(Tok); while (Tok.isAnnotation());
+
+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 =
- PP.getLangOpts().CompilingModule &&
- Tok.getIdentifierInfo()->getName() == PP.getLangOpts().CurrentModule;
+ case tok::l_paren:
+ ++ParenDepth;
+ if (Result.hasValue())
+ break;
+ if (!SuppressDiagnostic) {
+ auto Diag = PP.Diag(Tok.getLocation(), diag::err_pp_unexpected_after);
+ if (IdentifierInfo *LastII = LastTok.getIdentifierInfo())
+ Diag << LastII;
+ else
+ Diag << LastTok.getKind();
+ Diag << tok::l_paren << LastTok.getLocation();
+ SuppressDiagnostic = true;
+ }
+ continue;
- // Get ')'.
- PP.LexNonComment(Tok);
+ case tok::r_paren:
+ if (--ParenDepth > 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;
+ }
+ }
+
+ // Diagnose expected ')'.
+ if (!SuppressDiagnostic) {
+ if (auto Diag = PP.Diag(Tok.getLocation(), diag::err_pp_expected_after)) {
+ if (IdentifierInfo *LastII = LastTok.getIdentifierInfo())
+ Diag << LastII;
+ else
+ Diag << LastTok.getKind();
+ Diag << tok::r_paren << LastTok.getLocation();
+ }
+ PP.Diag(LParenLoc, diag::note_matching) << tok::l_paren;
+ SuppressDiagnostic = true;
+ }
}
+}
- return Result;
+/// \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;
+
+ PP.Diag(Tok.getLocation(), DiagID);
+ return nullptr;
}
/// ExpandBuiltinMacro - If an identifier token is read that is to be expanded
@@ -1601,84 +1677,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<bool>(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<bool>(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
@@ -1696,64 +1769,44 @@
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<diag::kind, 10> 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<diag::kind, 10> 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 PP.getLangOpts().CompilingModule && II &&
+ (II->getName() == PP.getLangOpts().CurrentModule);
+ });
} else if (II == Ident__MODULE__) {
// The current module as an identifier.
OS << getLangOpts().CurrentModule;
Index: include/clang/Basic/DiagnosticLexKinds.td
===================================================================
--- include/clang/Basic/DiagnosticLexKinds.td
+++ include/clang/Basic/DiagnosticLexKinds.td
@@ -401,6 +401,7 @@
def err_pp_expected_eol : Error<
"expected end of line in preprocessor expression">;
def err_pp_expected_after : Error<"missing %1 after %0">;
+def err_pp_unexpected_after : Error<"%1 not expected after %0">;
def err_pp_colon_without_question : Error<"':' without preceding '?'">;
def err_pp_division_by_zero : Error<
"division by zero in preprocessor expression">;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
