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

Serge Pavlov sepavloff at gmail.com
Thu Dec 11 11:43:57 PST 2014


Yes, if this test is run with '-triple i386-mingw32', the errorr is
observed, probably this test should have this run option.
Directive for suppressing this message was added to the test but it somehow
didn't work. Will look at it.

Thank you!

--Serge

2014-12-12 0:36 GMT+06:00 Aaron Ballman <aaron at aaronballman.com>:

> This appears to have broken at least one bot:
> http://bb.pgr.jp/builders/ninja-clang-x64-mingw64-RA/builds/5534
>
> I'm not certain whether the correct fix is to expect your new
> diagnostic, or to simply turn the diagnostic off for this test.
>
> ~Aaron
>
> On Thu, Dec 11, 2014 at 7: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.
> >
> > 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>;
> > +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/dbc4c115/attachment.html>


More information about the cfe-commits mailing list