Addressed reviewer's notes.
http://reviews.llvm.org/D6194
Files:
include/clang/Basic/DiagnosticGroups.td
include/clang/Basic/DiagnosticLexKinds.td
include/clang/Basic/IdentifierTable.h
lib/Basic/IdentifierTable.cpp
lib/Lex/PPDirectives.cpp
test/PCH/single-token-macro.c
test/Preprocessor/cxx_oper_keyword_ms_compat.cpp
test/Preprocessor/macro-reserved.c
test/Preprocessor/macro-reserved.cpp
test/Sema/thread-specifier.c
Index: include/clang/Basic/DiagnosticGroups.td
===================================================================
--- include/clang/Basic/DiagnosticGroups.td
+++ include/clang/Basic/DiagnosticGroups.td
@@ -336,6 +336,8 @@
// Preprocessor warnings.
def AmbiguousMacro : DiagGroup<"ambiguous-macro">;
+def KeywordAsMacro : DiagGroup<"keyword-macro">;
+def ReservedIdAsMacro : DiagGroup<"reserved-id-macro">;
// Just silence warnings about -Wstrict-aliasing for now.
def : DiagGroup<"strict-aliasing=0">;
Index: include/clang/Basic/DiagnosticLexKinds.td
===================================================================
--- include/clang/Basic/DiagnosticLexKinds.td
+++ include/clang/Basic/DiagnosticLexKinds.td
@@ -290,6 +290,14 @@
"expanding this definition of %0">;
def note_pp_ambiguous_macro_other : Note<
"other definition of %0">;
+def warn_pp_macro_hides_keyword : Warning<
+ "keyword or reserved identifier is hidden by macro definition">,
+ InGroup<KeywordAsMacro>;
+def warn_pp_macro_is_reserved_id : Warning<
+ "macro name is a keyword or reserved identifier">, InGroup<KeywordAsMacro>;
+def warn_pp_defundef_reserved_ident : Warning<
+ "reserved identifier is used as macro name">, DefaultIgnore,
+ InGroup<ReservedIdAsMacro>;
def pp_invalid_string_literal : Warning<
"invalid string literal, ignoring final '\\'">;
Index: include/clang/Basic/IdentifierTable.h
===================================================================
--- include/clang/Basic/IdentifierTable.h
+++ include/clang/Basic/IdentifierTable.h
@@ -249,6 +249,9 @@
}
bool isCPlusPlusOperatorKeyword() const { return IsCPPOperatorKeyword; }
+ /// \brief Return true if this token is a keyword in the specified language.
+ bool isKeyword(const LangOptions &LangOpts);
+
/// getFETokenInfo/setFETokenInfo - The language front-end is allowed to
/// associate arbitrary metadata with this token.
template<typename T>
Index: lib/Basic/IdentifierTable.cpp
===================================================================
--- lib/Basic/IdentifierTable.cpp
+++ lib/Basic/IdentifierTable.cpp
@@ -122,7 +122,7 @@
/// \brief Translates flags as specified in TokenKinds.def into keyword status
/// in the given language standard.
-static KeywordStatus GetKeywordStatus(const LangOptions &LangOpts,
+static KeywordStatus getKeywordStatus(const LangOptions &LangOpts,
unsigned Flags) {
if (Flags == KEYALL) return KS_Enabled;
if (LangOpts.CPlusPlus && (Flags & KEYCXX)) return KS_Enabled;
@@ -151,7 +151,7 @@
static void AddKeyword(StringRef Keyword,
tok::TokenKind TokenCode, unsigned Flags,
const LangOptions &LangOpts, IdentifierTable &Table) {
- KeywordStatus AddResult = GetKeywordStatus(LangOpts, Flags);
+ KeywordStatus AddResult = getKeywordStatus(LangOpts, Flags);
// Don't add this keyword under MSVCCompat.
if (LangOpts.MSVCCompat && (Flags & KEYNOMS))
@@ -209,6 +209,31 @@
LangOpts, *this);
}
+/// \brief Checks if the specified token kind represents a keyword in the
+/// specified language.
+/// \returns Status of the keyword in the language.
+static KeywordStatus getTokenKwStatus(const LangOptions &LangOpts,
+ tok::TokenKind K) {
+ switch (K) {
+#define KEYWORD(NAME, FLAGS) \
+ case tok::kw_##NAME: return getKeywordStatus(LangOpts, FLAGS);
+#include "clang/Basic/TokenKinds.def"
+ default: return KS_Disabled;
+ }
+}
+
+/// \brief Returns true if the identifier represents a keyword in the
+/// specified language.
+bool IdentifierInfo::isKeyword(const LangOptions &LangOpts) {
+ switch (getTokenKwStatus(LangOpts, getTokenID())) {
+ case KS_Enabled:
+ case KS_Extension:
+ return true;
+ default:
+ return false;
+ }
+}
+
tok::PPKeywordKind IdentifierInfo::getPPKeywordID() const {
// We use a perfect hash function here involving the length of the keyword,
// the first and third character. For preprocessor ID's there are no
Index: lib/Lex/PPDirectives.cpp
===================================================================
--- lib/Lex/PPDirectives.cpp
+++ lib/Lex/PPDirectives.cpp
@@ -100,6 +100,58 @@
} while (Tmp.isNot(tok::eod));
}
+/// \brief Enumerates possible cases of #define/#undef a reserved identifier.
+enum MacroDiag {
+ MD_NoWarn, //> Not a reserved identifier
+ MD_KeywordDef, //> Macro hides keyword, enabled by default
+ MD_KeywordUndef, //> #undef keyword, enabled by default
+ MD_WarnStrict //> Other reserved id, disabled by default
+};
+
+/// \brief Checks if the specified identifier is reserved in the specified
+/// language.
+/// This function does not check if the identifier is a keyword.
+static bool isReservedId(StringRef Text, const LangOptions &Lang) {
+ // C++ [macro.names], C11 7.1.3:
+ // All identifiers that begin with an underscore and either an uppercase
+ // letter or another underscore are always reserved for any use.
+ if (Text.size() >= 2 && Text[0] == '_' &&
+ (isUppercase(Text[1]) || Text[1] == '_'))
+ return true;
+ // C++ [global.names]
+ // Each name that contains a double underscore ... is reserved to the
+ // implementation for any use.
+ if (Lang.CPlusPlus) {
+ if (Text.find("__") != StringRef::npos)
+ return true;
+ }
+ return false;
+}
+
+static MacroDiag shouldWarnOnMacroDef(Preprocessor &PP, IdentifierInfo *II) {
+ const LangOptions &Lang = PP.getLangOpts();
+ StringRef Text = II->getName();
+ if (isReservedId(Text, Lang))
+ return MD_WarnStrict;
+ if (II->isKeyword(Lang))
+ return MD_KeywordDef;
+ if (Lang.CPlusPlus && (Text.equals("override") || Text.equals("final")))
+ return MD_KeywordDef;
+ return MD_NoWarn;
+}
+
+static MacroDiag shouldWarnOnMacroUndef(Preprocessor &PP, IdentifierInfo *II) {
+ const LangOptions &Lang = PP.getLangOpts();
+ if (II->isKeyword(Lang))
+ return MD_KeywordUndef;
+ StringRef Text = II->getName();
+ if (Lang.CPlusPlus && (Text.equals("override") || Text.equals("final")))
+ return MD_KeywordUndef;
+ if (isReservedId(Text, Lang))
+ return MD_WarnStrict;
+ return MD_NoWarn;
+}
+
bool Preprocessor::CheckMacroName(Token &MacroNameTok, MacroUse isDefineUndef) {
// Missing macro name?
if (MacroNameTok.is(tok::eod))
@@ -140,6 +192,23 @@
Diag(MacroNameTok, diag::ext_pp_undef_builtin_macro);
}
+ // Warn if defining/undefining reserved identifier including keywords.
+ SourceLocation MacroNameLoc = MacroNameTok.getLocation();
+ if (!SourceMgr.isInSystemHeader(MacroNameLoc) &&
+ (strcmp(SourceMgr.getBufferName(MacroNameLoc), "<built-in>") != 0)) {
+ MacroDiag D = MD_NoWarn;
+ if (isDefineUndef == MU_Define)
+ D = shouldWarnOnMacroDef(*this, II);
+ else if (isDefineUndef == MU_Undef)
+ D = shouldWarnOnMacroUndef(*this, II);
+ if (D == MD_KeywordDef)
+ Diag(MacroNameTok, diag::warn_pp_macro_hides_keyword);
+ if (D == MD_KeywordUndef)
+ Diag(MacroNameTok, diag::warn_pp_macro_is_reserved_id);
+ else if (D == MD_WarnStrict)
+ Diag(MacroNameTok, diag::warn_pp_defundef_reserved_ident);
+ }
+
// Okay, we got a good identifier.
return false;
}
Index: test/PCH/single-token-macro.c
===================================================================
--- test/PCH/single-token-macro.c
+++ test/PCH/single-token-macro.c
@@ -12,6 +12,8 @@
#ifndef HEADER
#define HEADER
+#pragma clang diagnostic ignored "-Wreserved-id-macro"
+
#ifdef __stdcall
// __stdcall is defined as __attribute__((__stdcall__)) for targeting mingw32.
#undef __stdcall
Index: test/Preprocessor/cxx_oper_keyword_ms_compat.cpp
===================================================================
--- test/Preprocessor/cxx_oper_keyword_ms_compat.cpp
+++ test/Preprocessor/cxx_oper_keyword_ms_compat.cpp
@@ -1,6 +1,8 @@
// RUN: %clang_cc1 %s -E -verify -fms-extensions
// expected-no-diagnostics
+#pragma clang diagnostic ignored "-Wkeyword-macro"
+
bool f() {
// Check that operators still work before redefining them.
#if compl 0 bitand 1
Index: test/Preprocessor/macro-reserved.c
===================================================================
--- /dev/null
+++ test/Preprocessor/macro-reserved.c
@@ -0,0 +1,29 @@
+// RUN: %clang_cc1 -fsyntax-only %s -verify
+
+#define for 0 // expected-warning {{keyword or reserved identifier is hidden by macro definition}}
+#define final 1
+#define __HAVE_X 0
+#define _HAVE_X 0
+#define X__Y
+
+#undef inline // expected-warning {{macro name is a keyword or reserved identifier}}
+#undef override
+#undef __cplusplus
+#undef _HAVE_X
+#undef X__Y
+
+#pragma clang diagnostic warning "-Wreserved-id-macro"
+
+#define switch if // expected-warning {{keyword or reserved identifier is hidden by macro definition}}
+#define final 1
+#define __HAVE_X 0 // expected-warning {{reserved identifier is used as macro name}}
+#define _HAVE_X 0 // expected-warning {{reserved identifier is used as macro name}}
+#define X__Y
+
+#undef inline // expected-warning {{macro name is a keyword or reserved identifier}}
+#undef override
+#undef __cplusplus // expected-warning {{reserved identifier is used as macro name}}
+#undef _HAVE_X // expected-warning {{reserved identifier is used as macro name}}
+#undef X__Y
+
+int x;
Index: test/Preprocessor/macro-reserved.cpp
===================================================================
--- /dev/null
+++ test/Preprocessor/macro-reserved.cpp
@@ -0,0 +1,29 @@
+// RUN: %clang_cc1 -fsyntax-only %s -verify
+
+#define for 0 // expected-warning {{keyword or reserved identifier is hidden by macro definition}}
+#define final 1 // expected-warning {{keyword or reserved identifier is hidden by macro definition}}
+#define __HAVE_X 0
+#define _HAVE_X 0
+#define X__Y
+
+#undef inline // expected-warning {{macro name is a keyword or reserved identifier}}
+#undef override // expected-warning {{macro name is a keyword or reserved identifier}}
+#undef __cplusplus
+#undef _HAVE_X
+#undef X__Y
+
+#pragma clang diagnostic warning "-Wreserved-id-macro"
+
+#define switch if // expected-warning {{keyword or reserved identifier is hidden by macro definition}}
+#define final 1 // expected-warning {{keyword or reserved identifier is hidden by macro definition}}
+#define __HAVE_X 0 // expected-warning {{reserved identifier is used as macro name}}
+#define _HAVE_X 0 // expected-warning {{reserved identifier is used as macro name}}
+#define X__Y // expected-warning {{reserved identifier is used as macro name}}
+
+#undef inline // expected-warning {{macro name is a keyword or reserved identifier}}
+#undef override // expected-warning {{macro name is a keyword or reserved identifier}}
+#undef __cplusplus // expected-warning {{reserved identifier is used as macro name}}
+#undef _HAVE_X // expected-warning {{reserved identifier is used as macro name}}
+#undef X__Y // expected-warning {{reserved identifier is used as macro name}}
+
+int x;
Index: test/Sema/thread-specifier.c
===================================================================
--- test/Sema/thread-specifier.c
+++ test/Sema/thread-specifier.c
@@ -5,6 +5,8 @@
// RUN: %clang_cc1 -triple i686-pc-linux-gnu -fsyntax-only -Wno-private-extern -verify -pedantic -x c++ %s -DCXX11 -D__thread=thread_local -std=c++11 -Wno-deprecated
// RUN: %clang_cc1 -triple i686-pc-linux-gnu -fsyntax-only -Wno-private-extern -verify -pedantic -x c++ %s -DC11 -D__thread=_Thread_local -std=c++11 -Wno-deprecated
+#pragma clang diagnostic ignored "-Wkeyword-macro"
+
#ifdef __cplusplus
// In C++, we define __private_extern__ to extern.
#undef __private_extern__
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits