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

Serge Pavlov sepavloff at gmail.com
Tue Dec 2 06:42:35 PST 2014


Hi Asiri,

Yes, it is my patch that caused the fail. The file
include/llvm/Config/config.h contains directive:

#undef const

which cause warning. The warning can be removed by specifying
-Wno-keyword-macro in command line or

#pragma clang diagnostic ignored "-Wkeyword-macro"

in source code.

If none of the methods is practical, please let me know, probably this
warning should be turned off by default.

Thanks,
--Serge

2014-12-02 19:02 GMT+06:00 Asiri Rathnayake <asiri.rathnayake at arm.com>:

> Hi Serge,
>
> Could you kindly check if the following failures (in sanitizer build) are
> caused by your patch:
>
>
> http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux/builds/14359/steps/
> bootstrap%20clang/logs/stdio
>
> It initially failed on my commit and I have committed a fix. The latest
> failures seem to be related to your patch.
>
> Thanks!
>
> --Asiri
>
> -----Original Message-----
> From: cfe-commits-bounces at cs.uiuc.edu
> [mailto:cfe-commits-bounces at cs.uiuc.edu] On Behalf Of Serge Pavlov
> Sent: 02 December 2014 11:06
> To: cfe-commits at cs.uiuc.edu
> Subject: r223114 - Emit warning if define or undef reserved identifier or
> keyword.
>
> Author: sepavloff
> Date: Tue Dec  2 05:06:09 2014
> New Revision: 223114
>
> URL: http://llvm.org/viewvc/llvm-project?rev=223114&view=rev
> Log:
> Emit warning if define or undef reserved identifier or keyword.
>
> Summary:
> This change implements warnings if macro name is identical to a keyword or
> reserved identifier. The warnings are different depending on the "danger"
> of the operation. Defining macro that replaces a keyword is on by default.
> Other cases produce warning that is off by default but can be turned on
> using option -Wreserved-id-macro.
>
> This change fixes PR11488.
>
> Reviewers: rnk
>
> Reviewed By: rnk
>
> Subscribers: rnk, cfe-commits
>
> Differential Revision: http://reviews.llvm.org/D6194
>
> 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/Diagnostic
> Groups.td?rev=223114&r1=223113&r2=223114&view=diff
>
> ============================================================================
> ==
> --- cfe/trunk/include/clang/Basic/DiagnosticGroups.td (original)
> +++ cfe/trunk/include/clang/Basic/DiagnosticGroups.td Tue Dec  2
> +++ 05:06:09 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/Diagnostic
> LexKinds.td?rev=223114&r1=223113&r2=223114&view=diff
>
> ============================================================================
> ==
> --- cfe/trunk/include/clang/Basic/DiagnosticLexKinds.td (original)
> +++ cfe/trunk/include/clang/Basic/DiagnosticLexKinds.td Tue Dec  2
> +++ 05:06:09 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>; 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/Identifier
> Table.h?rev=223114&r1=223113&r2=223114&view=diff
>
> ============================================================================
> ==
> --- cfe/trunk/include/clang/Basic/IdentifierTable.h (original)
> +++ cfe/trunk/include/clang/Basic/IdentifierTable.h Tue Dec  2 05:06:09
> +++ 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=223114&r1=223113&r2=223114&view=diff
>
> ============================================================================
> ==
> --- cfe/trunk/lib/Basic/IdentifierTable.cpp (original)
> +++ cfe/trunk/lib/Basic/IdentifierTable.cpp Tue Dec  2 05:06:09 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=2
> 23114&r1=223113&r2=223114&view=diff
>
> ============================================================================
> ==
> --- cfe/trunk/lib/Lex/PPDirectives.cpp (original)
> +++ cfe/trunk/lib/Lex/PPDirectives.cpp Tue Dec  2 05:06:09 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=223114&r1=223113&r2=223114&view=diff
>
> ============================================================================
> ==
> --- cfe/trunk/test/PCH/single-token-macro.c (original)
> +++ cfe/trunk/test/PCH/single-token-macro.c Tue Dec  2 05:06:09 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_key
> word_ms_compat.cpp?rev=223114&r1=223113&r2=223114&view=diff
>
> ============================================================================
> ==
> --- cfe/trunk/test/Preprocessor/cxx_oper_keyword_ms_compat.cpp (original)
> +++ cfe/trunk/test/Preprocessor/cxx_oper_keyword_ms_compat.cpp Tue Dec
> +++ 2 05:06:09 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?r
> ev=223114&r1=223113&r2=223114&view=diff
>
> ============================================================================
> ==
> --- cfe/trunk/test/Sema/thread-specifier.c (original)
> +++ cfe/trunk/test/Sema/thread-specifier.c Tue Dec  2 05:06:09 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/20141202/b78ae2e4/attachment.html>


More information about the cfe-commits mailing list