Thank you for feedback. I removed warning on #undef keyword, and changed wording in r224100.
--Serge 2014-12-12 3:21 GMT+06:00 Richard Smith <[email protected]>: > > On Thu, Dec 11, 2014 at 4:18 AM, Serge Pavlov <[email protected]> wrote: > >> Author: sepavloff >> Date: Thu Dec 11 06:18:08 2014 >> New Revision: 224012 >> >> URL: http://llvm.org/viewvc/llvm-project?rev=224012&view=rev >> Log: >> Emit warning if define or undef reserved identifier or keyword. >> > > Warning on a #undef of a keyword doesn't seem useful, and it causes > real-world problems because a lot of configure scripts generate a config.h > that #undef's keywords (in particular, 'const' often gets this treatment). > Please either remove the warning on #undef of a keyword or put it under a > different -W flag. > > Recommit of r223114, reverted in r223120. >> >> >> Modified: >> cfe/trunk/include/clang/Basic/DiagnosticGroups.td >> cfe/trunk/include/clang/Basic/DiagnosticLexKinds.td >> cfe/trunk/include/clang/Basic/IdentifierTable.h >> cfe/trunk/lib/Basic/IdentifierTable.cpp >> cfe/trunk/lib/Lex/PPDirectives.cpp >> cfe/trunk/test/PCH/single-token-macro.c >> cfe/trunk/test/Preprocessor/cxx_oper_keyword_ms_compat.cpp >> cfe/trunk/test/Sema/thread-specifier.c >> >> Modified: cfe/trunk/include/clang/Basic/DiagnosticGroups.td >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticGroups.td?rev=224012&r1=224011&r2=224012&view=diff >> >> ============================================================================== >> --- cfe/trunk/include/clang/Basic/DiagnosticGroups.td (original) >> +++ cfe/trunk/include/clang/Basic/DiagnosticGroups.td Thu Dec 11 06:18:08 >> 2014 >> @@ -337,6 +337,8 @@ def : DiagGroup<"sequence-point", [Unseq >> >> // 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">; >> >> Modified: cfe/trunk/include/clang/Basic/DiagnosticLexKinds.td >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticLexKinds.td?rev=224012&r1=224011&r2=224012&view=diff >> >> ============================================================================== >> --- cfe/trunk/include/clang/Basic/DiagnosticLexKinds.td (original) >> +++ cfe/trunk/include/clang/Basic/DiagnosticLexKinds.td Thu Dec 11 >> 06:18:08 2014 >> @@ -290,6 +290,14 @@ def note_pp_ambiguous_macro_chosen : Not >> "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>; >> > > "keyword or reserved identifier" is very imprecise wording. Please figure > out what case we're in, and specify that in the diagnostic. These two cases > are not the same, and should really be under different -W flags. > > >> +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 '\\'">; >> >> Modified: cfe/trunk/include/clang/Basic/IdentifierTable.h >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/IdentifierTable.h?rev=224012&r1=224011&r2=224012&view=diff >> >> ============================================================================== >> --- cfe/trunk/include/clang/Basic/IdentifierTable.h (original) >> +++ cfe/trunk/include/clang/Basic/IdentifierTable.h Thu Dec 11 06:18:08 >> 2014 >> @@ -249,6 +249,9 @@ public: >> } >> 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> >> >> Modified: cfe/trunk/lib/Basic/IdentifierTable.cpp >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/IdentifierTable.cpp?rev=224012&r1=224011&r2=224012&view=diff >> >> ============================================================================== >> --- cfe/trunk/lib/Basic/IdentifierTable.cpp (original) >> +++ cfe/trunk/lib/Basic/IdentifierTable.cpp Thu Dec 11 06:18:08 2014 >> @@ -122,7 +122,7 @@ namespace { >> >> /// \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 KeywordStatus GetKeywordStatus(co >> 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 @@ void IdentifierTable::AddKeywords(const >> 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 >> >> Modified: cfe/trunk/lib/Lex/PPDirectives.cpp >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/PPDirectives.cpp?rev=224012&r1=224011&r2=224012&view=diff >> >> ============================================================================== >> --- cfe/trunk/lib/Lex/PPDirectives.cpp (original) >> +++ cfe/trunk/lib/Lex/PPDirectives.cpp Thu Dec 11 06:18:08 2014 >> @@ -100,6 +100,58 @@ void Preprocessor::DiscardUntilEndOfDire >> } 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 @@ bool Preprocessor::CheckMacroName(Token >> 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; >> } >> >> Modified: cfe/trunk/test/PCH/single-token-macro.c >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/PCH/single-token-macro.c?rev=224012&r1=224011&r2=224012&view=diff >> >> ============================================================================== >> --- cfe/trunk/test/PCH/single-token-macro.c (original) >> +++ cfe/trunk/test/PCH/single-token-macro.c Thu Dec 11 06:18:08 2014 >> @@ -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 >> >> Modified: cfe/trunk/test/Preprocessor/cxx_oper_keyword_ms_compat.cpp >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Preprocessor/cxx_oper_keyword_ms_compat.cpp?rev=224012&r1=224011&r2=224012&view=diff >> >> ============================================================================== >> --- cfe/trunk/test/Preprocessor/cxx_oper_keyword_ms_compat.cpp (original) >> +++ cfe/trunk/test/Preprocessor/cxx_oper_keyword_ms_compat.cpp Thu Dec 11 >> 06:18:08 2014 >> @@ -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 >> >> Modified: cfe/trunk/test/Sema/thread-specifier.c >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/thread-specifier.c?rev=224012&r1=224011&r2=224012&view=diff >> >> ============================================================================== >> --- cfe/trunk/test/Sema/thread-specifier.c (original) >> +++ cfe/trunk/test/Sema/thread-specifier.c Thu Dec 11 06:18:08 2014 >> @@ -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 >> > >
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
