r224012 - Emit warning if define or undef reserved identifier or keyword.
Serge Pavlov
sepavloff at gmail.com
Fri Dec 12 02:48:40 PST 2014
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 <richard at metafoo.co.uk>:
>
> On Thu, Dec 11, 2014 at 4:18 AM, Serge Pavlov <sepavloff at gmail.com> 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
>> cfe-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20141212/40f360d3/attachment.html>
More information about the cfe-commits
mailing list