[cfe-commits] r164892 - in /cfe/trunk: include/clang/AST/ASTContext.h include/clang/AST/CommentSema.h include/clang/AST/RawCommentList.h include/clang/Lex/Preprocessor.h lib/AST/ASTContext.cpp lib/AST/CommentSema.cpp lib/AST/RawCommentList.cpp li

Matthieu Monrocq matthieu.monrocq at gmail.com
Sat Sep 29 06:55:18 PDT 2012


On Sat, Sep 29, 2012 at 1:40 PM, Dmitri Gribenko <gribozavr at gmail.com> wrote:
> Author: gribozavr
> Date: Sat Sep 29 06:40:46 2012
> New Revision: 164892
>
> URL: http://llvm.org/viewvc/llvm-project?rev=164892&view=rev
> Log:
> Move the 'find macro by spelling' infrastructure to the Preprocessor class and
> use it to suggest appropriate macro for __attribute__((deprecated)) in
> -Wdocumentation-deprecated-sync.
>
> Modified:
>     cfe/trunk/include/clang/AST/ASTContext.h
>     cfe/trunk/include/clang/AST/CommentSema.h
>     cfe/trunk/include/clang/AST/RawCommentList.h
>     cfe/trunk/include/clang/Lex/Preprocessor.h
>     cfe/trunk/lib/AST/ASTContext.cpp
>     cfe/trunk/lib/AST/CommentSema.cpp
>     cfe/trunk/lib/AST/RawCommentList.cpp
>     cfe/trunk/lib/Lex/Preprocessor.cpp
>     cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp
>     cfe/trunk/lib/Sema/SemaDecl.cpp
>     cfe/trunk/test/Sema/warn-documentation-fixits.cpp
>     cfe/trunk/tools/libclang/CIndex.cpp
>     cfe/trunk/unittests/AST/CommentParser.cpp
>
> Modified: cfe/trunk/include/clang/AST/ASTContext.h
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/ASTContext.h?rev=164892&r1=164891&r2=164892&view=diff
> ==============================================================================
> --- cfe/trunk/include/clang/AST/ASTContext.h (original)
> +++ cfe/trunk/include/clang/AST/ASTContext.h Sat Sep 29 06:40:46 2012
> @@ -532,7 +532,11 @@
>
>    /// Return parsed documentation comment attached to a given declaration.
>    /// Returns NULL if no comment is attached.
> -  comments::FullComment *getCommentForDecl(const Decl *D) const;
> +  ///
> +  /// \param PP the Preprocessor used with this TU.  Could be NULL if
> +  /// preprocessor is not available.
> +  comments::FullComment *getCommentForDecl(const Decl *D,
> +                                           const Preprocessor *PP) const;
>
>  private:
>    mutable comments::CommandTraits CommentCommandTraits;
>
> Modified: cfe/trunk/include/clang/AST/CommentSema.h
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/CommentSema.h?rev=164892&r1=164891&r2=164892&view=diff
> ==============================================================================
> --- cfe/trunk/include/clang/AST/CommentSema.h (original)
> +++ cfe/trunk/include/clang/AST/CommentSema.h Sat Sep 29 06:40:46 2012
> @@ -25,6 +25,7 @@
>  namespace clang {
>  class Decl;
>  class SourceMgr;
> +class Preprocessor;
>
>  namespace comments {
>  class CommandTraits;
> @@ -43,6 +44,8 @@
>
>    CommandTraits &Traits;
>
> +  const Preprocessor *PP;
> +
>    /// Information about the declaration this comment is attached to.
>    DeclInfo *ThisDeclInfo;
>
> @@ -68,7 +71,8 @@
>
>  public:
>    Sema(llvm::BumpPtrAllocator &Allocator, const SourceManager &SourceMgr,
> -       DiagnosticsEngine &Diags, CommandTraits &Traits);
> +       DiagnosticsEngine &Diags, CommandTraits &Traits,
> +       const Preprocessor *PP);
>
>    void setDecl(const Decl *D);
>
>
> Modified: cfe/trunk/include/clang/AST/RawCommentList.h
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/RawCommentList.h?rev=164892&r1=164891&r2=164892&view=diff
> ==============================================================================
> --- cfe/trunk/include/clang/AST/RawCommentList.h (original)
> +++ cfe/trunk/include/clang/AST/RawCommentList.h Sat Sep 29 06:40:46 2012
> @@ -18,6 +18,7 @@
>  class ASTContext;
>  class ASTReader;
>  class Decl;
> +class Preprocessor;
>
>  namespace comments {
>    class FullComment;
> @@ -114,7 +115,8 @@
>    }
>
>    /// Parse the comment, assuming it is attached to decl \c D.
> -  comments::FullComment *parse(const ASTContext &Context, const Decl *D) const;
> +  comments::FullComment *parse(const ASTContext &Context,
> +                               const Preprocessor *PP, const Decl *D) const;
>
>  private:
>    SourceRange Range;
>
> Modified: cfe/trunk/include/clang/Lex/Preprocessor.h
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Lex/Preprocessor.h?rev=164892&r1=164891&r2=164892&view=diff
> ==============================================================================
> --- cfe/trunk/include/clang/Lex/Preprocessor.h (original)
> +++ cfe/trunk/include/clang/Lex/Preprocessor.h Sat Sep 29 06:40:46 2012
> @@ -55,6 +55,27 @@
>  class PreprocessingRecord;
>  class ModuleLoader;
>
> +/// \brief Stores token information for comparing actual tokens with
> +/// predefined values.  Only handles simple tokens and identifiers.
> +class TokenValue {
> +  tok::TokenKind Kind;
> +  IdentifierInfo *II;
> +
> +public:
> +  TokenValue(tok::TokenKind Kind) : Kind(Kind), II(0) {
> +    assert(Kind != tok::raw_identifier && "Raw identifiers are not supported.");
> +    assert(Kind != tok::identifier &&
> +           "Identifiers should be created by TokenValue(IdentifierInfo *)");
> +    assert(!tok::isLiteral(Kind) && "Literals are not supported.");
> +    assert(!tok::isAnnotation(Kind) && "Annotations are not supported.");
> +  }
> +  TokenValue(IdentifierInfo *II) : Kind(tok::identifier), II(II) {}
> +  bool operator==(const Token &Tok) const {
> +    return Tok.getKind() == Kind &&
> +        (!II || II == Tok.getIdentifierInfo());
> +  }
> +};
> +
>  /// Preprocessor - This object engages in a tight little dance with the lexer to
>  /// efficiently preprocess tokens.  Lexers know only about tokens within a
>  /// single source file, and don't know anything about preprocessor-level issues
> @@ -491,6 +512,12 @@
>    macro_iterator macro_begin(bool IncludeExternalMacros = true) const;
>    macro_iterator macro_end(bool IncludeExternalMacros = true) const;
>
> +  /// \brief Return the name of the macro defined before \p Loc that has
> +  /// spelling \p Tokens.  If there are multiple macros with same spelling,
> +  /// return the last one defined.
> +  StringRef getLastMacroWithSpelling(SourceLocation Loc,
> +                                     ArrayRef<TokenValue> Tokens) const;
> +
>    const std::string &getPredefines() const { return Predefines; }
>    /// setPredefines - Set the predefines for this Preprocessor.  These
>    /// predefines are automatically injected when parsing the main file.
>
> Modified: cfe/trunk/lib/AST/ASTContext.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTContext.cpp?rev=164892&r1=164891&r2=164892&view=diff
> ==============================================================================
> --- cfe/trunk/lib/AST/ASTContext.cpp (original)
> +++ cfe/trunk/lib/AST/ASTContext.cpp Sat Sep 29 06:40:46 2012
> @@ -355,7 +355,9 @@
>    return RC;
>  }
>
> -comments::FullComment *ASTContext::getCommentForDecl(const Decl *D) const {
> +comments::FullComment *ASTContext::getCommentForDecl(
> +                                              const Decl *D,
> +                                              const Preprocessor *PP) const {
>    D = adjustDeclToTemplate(D);
>    const Decl *Canonical = D->getCanonicalDecl();
>    llvm::DenseMap<const Decl *, comments::FullComment *>::iterator Pos =
> @@ -373,9 +375,9 @@
>    // because comments can contain references to parameter names which can be
>    // different across redeclarations.
>    if (D != OriginalDecl)
> -    return getCommentForDecl(OriginalDecl);
> +    return getCommentForDecl(OriginalDecl, PP);
>
> -  comments::FullComment *FC = RC->parse(*this, D);
> +  comments::FullComment *FC = RC->parse(*this, PP, D);
>    ParsedComments[Canonical] = FC;
>    return FC;
>  }
>
> Modified: cfe/trunk/lib/AST/CommentSema.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/CommentSema.cpp?rev=164892&r1=164891&r2=164892&view=diff
> ==============================================================================
> --- cfe/trunk/lib/AST/CommentSema.cpp (original)
> +++ cfe/trunk/lib/AST/CommentSema.cpp Sat Sep 29 06:40:46 2012
> @@ -13,7 +13,9 @@
>  #include "clang/AST/Decl.h"
>  #include "clang/AST/DeclTemplate.h"
>  #include "clang/Basic/SourceManager.h"
> +#include "clang/Lex/Preprocessor.h"
>  #include "llvm/ADT/StringSwitch.h"
> +#include "llvm/ADT/SmallString.h"
>
>  namespace clang {
>  namespace comments {
> @@ -23,9 +25,10 @@
>  } // unnamed namespace
>
>  Sema::Sema(llvm::BumpPtrAllocator &Allocator, const SourceManager &SourceMgr,
> -           DiagnosticsEngine &Diags, CommandTraits &Traits) :
> +           DiagnosticsEngine &Diags, CommandTraits &Traits,
> +           const Preprocessor *PP) :
>      Allocator(Allocator), SourceMgr(SourceMgr), Diags(Diags), Traits(Traits),
> -    ThisDeclInfo(NULL), BriefCommand(NULL), ReturnsCommand(NULL) {
> +    PP(PP), ThisDeclInfo(NULL), BriefCommand(NULL), ReturnsCommand(NULL) {
>  }
>
>  void Sema::setDecl(const Decl *D) {
> @@ -527,10 +530,25 @@
>          FD->doesThisDeclarationHaveABody())
>        return;
>
> +    StringRef AttributeSpelling = "__attribute__((deprecated))";
> +    if (PP) {
> +      TokenValue Tokens[] = {
> +        tok::kw___attribute, tok::l_paren, tok::l_paren,
> +        PP->getIdentifierInfo("deprecated"),
> +        tok::r_paren, tok::r_paren
> +      };
> +      StringRef MacroName = PP->getLastMacroWithSpelling(FD->getLocation(),
> +                                                         Tokens);
> +      if (!MacroName.empty())
> +        AttributeSpelling = MacroName;
> +    }
> +
> +    SmallString<64> TextToInsert(" ");
> +    TextToInsert += AttributeSpelling;
>      Diag(FD->getLocEnd(),
>           diag::note_add_deprecation_attr)
>        << FixItHint::CreateInsertion(FD->getLocEnd().getLocWithOffset(1),
> -                                    " __attribute__((deprecated))");
> +                                    TextToInsert);
>    }
>  }
>
>
> Modified: cfe/trunk/lib/AST/RawCommentList.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/RawCommentList.cpp?rev=164892&r1=164891&r2=164892&view=diff
> ==============================================================================
> --- cfe/trunk/lib/AST/RawCommentList.cpp (original)
> +++ cfe/trunk/lib/AST/RawCommentList.cpp Sat Sep 29 06:40:46 2012
> @@ -159,6 +159,7 @@
>  }
>
>  comments::FullComment *RawComment::parse(const ASTContext &Context,
> +                                         const Preprocessor *PP,
>                                           const Decl *D) const {
>    // Make sure that RawText is valid.
>    getRawText(Context.getSourceManager());
> @@ -168,7 +169,8 @@
>                      RawText.begin(), RawText.end());
>    comments::Sema S(Context.getAllocator(), Context.getSourceManager(),
>                     Context.getDiagnostics(),
> -                   Context.getCommentCommandTraits());
> +                   Context.getCommentCommandTraits(),
> +                   PP);
>    S.setDecl(D);
>    comments::Parser P(L, S, Context.getAllocator(), Context.getSourceManager(),
>                       Context.getDiagnostics(),
>
> Modified: cfe/trunk/lib/Lex/Preprocessor.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/Preprocessor.cpp?rev=164892&r1=164891&r2=164892&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Lex/Preprocessor.cpp (original)
> +++ cfe/trunk/lib/Lex/Preprocessor.cpp Sat Sep 29 06:40:46 2012
> @@ -285,6 +285,39 @@
>    return Macros.end();
>  }
>
> +/// \brief Compares macro tokens with a specified token value sequence.
> +static bool MacroDefinitionEquals(const MacroInfo *MI,
> +                                  llvm::ArrayRef<TokenValue> Tokens) {
> +  return Tokens.size() == MI->getNumTokens() &&
> +      std::equal(Tokens.begin(), Tokens.end(), MI->tokens_begin());
> +}
> +
> +StringRef Preprocessor::getLastMacroWithSpelling(
> +                                    SourceLocation Loc,
> +                                    ArrayRef<TokenValue> Tokens) const {
> +  SourceLocation BestLocation;
> +  StringRef BestSpelling;
> +  for (Preprocessor::macro_iterator I = macro_begin(), E = macro_end();
> +       I != E; ++I) {
> +    if (!I->second->isObjectLike())
> +      continue;
> +    const MacroInfo *MI = I->second->findDefinitionAtLoc(Loc, SourceMgr);
> +    if (!MI)
> +      continue;
> +    if (!MacroDefinitionEquals(MI, Tokens))
> +      continue;
> +    SourceLocation Location = I->second->getDefinitionLoc();
> +    // Choose the macro defined latest.
> +    if (BestLocation.isInvalid() ||
> +        (Location.isValid() &&
> +         SourceMgr.isBeforeInTranslationUnit(BestLocation, Location))) {
> +      BestLocation = Location;
> +      BestSpelling = I->first->getName();
> +    }
> +  }
> +  return BestSpelling;
> +}
> +
>  void Preprocessor::recomputeCurLexerKind() {
>    if (CurLexer)
>      CurLexerKind = CLK_Lexer;
>
> Modified: cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp?rev=164892&r1=164891&r2=164892&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp (original)
> +++ cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp Sat Sep 29 06:40:46 2012
> @@ -41,6 +41,7 @@
>  #include "llvm/ADT/FoldingSet.h"
>  #include "llvm/ADT/ImmutableMap.h"
>  #include "llvm/ADT/PostOrderIterator.h"
> +#include "llvm/ADT/SmallString.h"
>  #include "llvm/ADT/SmallVector.h"
>  #include "llvm/ADT/StringRef.h"
>  #include "llvm/Support/Casting.h"
> @@ -679,61 +680,6 @@
>    return true;
>  }
>
> -/// \brief Stores token information for comparing actual tokens with
> -/// predefined values. Only handles simple tokens and identifiers.
> -class TokenValue {
> -  tok::TokenKind Kind;
> -  IdentifierInfo *II;
> -
> -public:
> -  TokenValue(tok::TokenKind Kind) : Kind(Kind), II(0) {
> -    assert(Kind != tok::raw_identifier && "Raw identifiers are not supported.");
> -    assert(Kind != tok::identifier &&
> -           "Identifiers should be created by TokenValue(IdentifierInfo *)");
> -    assert(!tok::isLiteral(Kind) && "Literals are not supported.");
> -    assert(!tok::isAnnotation(Kind) && "Annotations are not supported.");
> -  }
> -  TokenValue(IdentifierInfo *II) : Kind(tok::identifier), II(II) {}
> -  bool operator==(const Token &Tok) const {
> -    return Tok.getKind() == Kind &&
> -        (!II || II == Tok.getIdentifierInfo());
> -  }
> -};
> -
> -/// \brief Compares macro tokens with a specified token value sequence.
> -static bool MacroDefinitionEquals(const MacroInfo *MI,
> -                                  llvm::ArrayRef<TokenValue> Tokens) {
> -  return Tokens.size() == MI->getNumTokens() &&
> -      std::equal(Tokens.begin(), Tokens.end(), MI->tokens_begin());
> -}
> -
> -static std::string GetSuitableSpelling(Preprocessor &PP, SourceLocation L,
> -                                       llvm::ArrayRef<TokenValue> Tokens,
> -                                       const char *Spelling) {
> -  SourceManager &SM = PP.getSourceManager();
> -  SourceLocation BestLocation;
> -  std::string BestSpelling = Spelling;
> -  for (Preprocessor::macro_iterator I = PP.macro_begin(), E = PP.macro_end();
> -       I != E; ++I) {
> -    if (!I->second->isObjectLike())
> -      continue;
> -    const MacroInfo *MI = I->second->findDefinitionAtLoc(L, SM);
> -    if (!MI)
> -      continue;
> -    if (!MacroDefinitionEquals(MI, Tokens))
> -      continue;
> -    SourceLocation Location = I->second->getDefinitionLoc();
> -    // Choose the macro defined latest.
> -    if (BestLocation.isInvalid() ||
> -        (Location.isValid() &&
> -         SM.isBeforeInTranslationUnit(BestLocation, Location))) {
> -      BestLocation = Location;
> -      BestSpelling = I->first->getName();
> -    }
> -  }
> -  return BestSpelling;
> -}
> -
>  namespace {
>    class FallthroughMapper : public RecursiveASTVisitor<FallthroughMapper> {
>    public:
> @@ -914,11 +860,15 @@
>              tok::coloncolon, PP.getIdentifierInfo("fallthrough"),
>              tok::r_square, tok::r_square
>            };
> -          std::string AnnotationSpelling = GetSuitableSpelling(
> -              PP, L, Tokens, "[[clang::fallthrough]]");
> +          StringRef AnnotationSpelling = "[[clang::fallthrough]]";
> +          StringRef MacroName = PP.getLastMacroWithSpelling(L, Tokens);
> +          if (!MacroName.empty())
> +            AnnotationSpelling = MacroName;
> +          SmallString<64> TextToInsert(AnnotationSpelling);
> +          TextToInsert += "; ";
>            S.Diag(L, diag::note_insert_fallthrough_fixit) <<
>                AnnotationSpelling <<
> -              FixItHint::CreateInsertion(L, AnnotationSpelling + "; ");
> +              FixItHint::CreateInsertion(L, TextToInsert);
>          }
>        }
>        S.Diag(L, diag::note_insert_break_fixit) <<
>
> Modified: cfe/trunk/lib/Sema/SemaDecl.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDecl.cpp?rev=164892&r1=164891&r2=164892&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Sema/SemaDecl.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaDecl.cpp Sat Sep 29 06:40:46 2012
> @@ -7266,7 +7266,7 @@
>      // the lookahead in the lexer: we've consumed the semicolon and looked
>      // ahead through comments.
>      for (unsigned i = 0; i != NumDecls; ++i)
> -      Context.getCommentForDecl(Group[i]);
> +      Context.getCommentForDecl(Group[i], &PP);
>    }
>  }
>
>
> Modified: cfe/trunk/test/Sema/warn-documentation-fixits.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/warn-documentation-fixits.cpp?rev=164892&r1=164891&r2=164892&view=diff
> ==============================================================================
> --- cfe/trunk/test/Sema/warn-documentation-fixits.cpp (original)
> +++ cfe/trunk/test/Sema/warn-documentation-fixits.cpp Sat Sep 29 06:40:46 2012
> @@ -51,6 +51,12 @@
>    }
>  };
>
> +#define MY_ATTR_DEPRECATED __attribute__((deprecated))
> +
> +// expected-warning at +1 {{declaration is marked with '\deprecated' command but does not have a deprecation attribute}} expected-note at +2 {{add a deprecation attribute to the declaration to silence this warning}}
> +/// \deprecated
> +void test_deprecated_9(int a);
> +
>  // CHECK: fix-it:"{{.*}}":{5:12-5:22}:"a"
>  // CHECK: fix-it:"{{.*}}":{9:12-9:15}:"aaa"
>  // CHECK: fix-it:"{{.*}}":{13:13-13:23}:"T"
> @@ -61,4 +67,5 @@
>  // CHECK: fix-it:"{{.*}}":{38:27-38:27}:" __attribute__((deprecated))"
>  // CHECK: fix-it:"{{.*}}":{46:27-46:27}:" __attribute__((deprecated))"
>  // CHECK: fix-it:"{{.*}}":{50:27-50:27}:" __attribute__((deprecated))"
> +// CHECK: fix-it:"{{.*}}":{58:30-58:30}:" MY_ATTR_DEPRECATED"
>
>
> Modified: cfe/trunk/tools/libclang/CIndex.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/libclang/CIndex.cpp?rev=164892&r1=164891&r2=164892&view=diff
> ==============================================================================
> --- cfe/trunk/tools/libclang/CIndex.cpp (original)
> +++ cfe/trunk/tools/libclang/CIndex.cpp Sat Sep 29 06:40:46 2012
> @@ -5810,7 +5810,7 @@
>
>    const Decl *D = getCursorDecl(C);
>    const ASTContext &Context = getCursorContext(C);
> -  const comments::FullComment *FC = Context.getCommentForDecl(D);
> +  const comments::FullComment *FC = Context.getCommentForDecl(D, /*PP=*/ NULL);
>
>    return cxcomment::createCXComment(FC, getCursorTU(C));
>  }
>
> Modified: cfe/trunk/unittests/AST/CommentParser.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/AST/CommentParser.cpp?rev=164892&r1=164891&r2=164892&view=diff
> ==============================================================================
> --- cfe/trunk/unittests/AST/CommentParser.cpp (original)
> +++ cfe/trunk/unittests/AST/CommentParser.cpp Sat Sep 29 06:40:46 2012
> @@ -59,7 +59,7 @@
>
>    Lexer L(Allocator, Traits, Begin, Source, Source + strlen(Source));
>
> -  Sema S(Allocator, SourceMgr, Diags, Traits);
> +  Sema S(Allocator, SourceMgr, Diags, Traits, /*PP=*/ NULL);
>    Parser P(L, S, Allocator, SourceMgr, Diags, Traits);
>    FullComment *FC = P.parseFullComment();
>
>

It's great to see Alexander's work reused so quickly after its
introduction. I wonder if we have other "attributes" or new keywords
(C11, C++11) that would benefit from this kind of check as well.

Being primarily a C++ developer I can think of "final", "override" as
well as "[[noreturn]]" and "__attribute__((noreturn))" at least,
however I am not sure they are suggested yet.

-- Matthieu.



More information about the cfe-commits mailing list