r224012 - Emit warning if define or undef reserved identifier or keyword.

[Richard Smith]

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/20141211/72583cfc/attachment.html>