r368732 - [clang] Refactor doc comments to Decls attribution

Jan Korous via cfe-commits cfe-commits at lists.llvm.org
Tue Aug 13 11:11:44 PDT 2019


Author: jkorous
Date: Tue Aug 13 11:11:44 2019
New Revision: 368732

URL: http://llvm.org/viewvc/llvm-project?rev=368732&view=rev
Log:
[clang] Refactor doc comments to Decls attribution

- Create ASTContext::attachCommentsToJustParsedDecls so we don't have to load external comments in Sema when trying to attach existing comments to just parsed Decls.
- Keep comments ordered and cache their decomposed location - faster SourceLoc-based searching.
- Optimize work with redeclarations.
- Keep one comment per redeclaration chain (represented by canonical Decl) instead of comment per redeclaration.
- For redeclaration chains with no comment attached keep just the last declaration in chain that had no comment instead of every comment-less redeclaration.

Differential Revision: https://reviews.llvm.org/D65301

Added:
    cfe/trunk/test/Index/comment-redeclarations.cpp
Modified:
    cfe/trunk/include/clang/AST/ASTContext.h
    cfe/trunk/include/clang/AST/RawCommentList.h
    cfe/trunk/lib/AST/ASTContext.cpp
    cfe/trunk/lib/AST/RawCommentList.cpp
    cfe/trunk/lib/Sema/SemaDecl.cpp
    cfe/trunk/lib/Serialization/ASTReader.cpp
    cfe/trunk/lib/Serialization/ASTWriter.cpp

Modified: cfe/trunk/include/clang/AST/ASTContext.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/ASTContext.h?rev=368732&r1=368731&r2=368732&view=diff
==============================================================================
--- cfe/trunk/include/clang/AST/ASTContext.h (original)
+++ cfe/trunk/include/clang/AST/ASTContext.h Tue Aug 13 11:11:44 2019
@@ -729,71 +729,49 @@ public:
   /// True if comments are already loaded from ExternalASTSource.
   mutable bool CommentsLoaded = false;
 
-  class RawCommentAndCacheFlags {
-  public:
-    enum Kind {
-      /// We searched for a comment attached to the particular declaration, but
-      /// didn't find any.
-      ///
-      /// getRaw() == 0.
-      NoCommentInDecl = 0,
-
-      /// We have found a comment attached to this particular declaration.
-      ///
-      /// getRaw() != 0.
-      FromDecl,
-
-      /// This declaration does not have an attached comment, and we have
-      /// searched the redeclaration chain.
-      ///
-      /// If getRaw() == 0, the whole redeclaration chain does not have any
-      /// comments.
-      ///
-      /// If getRaw() != 0, it is a comment propagated from other
-      /// redeclaration.
-      FromRedecl
-    };
-
-    Kind getKind() const LLVM_READONLY {
-      return Data.getInt();
-    }
-
-    void setKind(Kind K) {
-      Data.setInt(K);
-    }
-
-    const RawComment *getRaw() const LLVM_READONLY {
-      return Data.getPointer();
-    }
-
-    void setRaw(const RawComment *RC) {
-      Data.setPointer(RC);
-    }
-
-    const Decl *getOriginalDecl() const LLVM_READONLY {
-      return OriginalDecl;
-    }
-
-    void setOriginalDecl(const Decl *Orig) {
-      OriginalDecl = Orig;
-    }
-
-  private:
-    llvm::PointerIntPair<const RawComment *, 2, Kind> Data;
-    const Decl *OriginalDecl;
-  };
+  /// Mapping from declaration to directly attached comment.
+  ///
+  /// Raw comments are owned by Comments list.  This mapping is populated
+  /// lazily.
+  mutable llvm::DenseMap<const Decl *, const RawComment *> DeclRawComments;
 
-  /// Mapping from declarations to comments attached to any
-  /// redeclaration.
+  /// Mapping from canonical declaration to the first redeclaration in chain
+  /// that has a comment attached.
   ///
   /// Raw comments are owned by Comments list.  This mapping is populated
   /// lazily.
-  mutable llvm::DenseMap<const Decl *, RawCommentAndCacheFlags> RedeclComments;
+  mutable llvm::DenseMap<const Decl *, const Decl *> RedeclChainComments;
+
+  /// Keeps track of redeclaration chains that don't have any comment attached.
+  /// Mapping from canonical declaration to redeclaration chain that has no
+  /// comments attached to any redeclaration. Specifically it's mapping to
+  /// the last redeclaration we've checked.
+  ///
+  /// Shall not contain declarations that have comments attached to any
+  /// redeclaration in their chain.
+  mutable llvm::DenseMap<const Decl *, const Decl *> CommentlessRedeclChains;
 
   /// Mapping from declarations to parsed comments attached to any
   /// redeclaration.
   mutable llvm::DenseMap<const Decl *, comments::FullComment *> ParsedComments;
 
+  /// Attaches \p Comment to \p OriginalD and to its redeclaration chain
+  /// and removes the redeclaration chain from the set of commentless chains.
+  ///
+  /// Don't do anything if a comment has already been attached to \p OriginalD
+  /// or its redeclaration chain.
+  void cacheRawCommentForDecl(const Decl &OriginalD,
+                              const RawComment &Comment) const;
+
+  /// \returns searches \p CommentsInFile for doc comment for \p D.
+  ///
+  /// \p RepresentativeLocForDecl is used as a location for searching doc
+  /// comments. \p CommentsInFile is a mapping offset -> comment of files in the
+  /// same file where \p RepresentativeLocForDecl is.
+  RawComment *getRawCommentForDeclNoCacheImpl(
+      const Decl *D, const SourceLocation RepresentativeLocForDecl,
+      const std::map<unsigned, RawComment *> &CommentsInFile) const;
+
   /// Return the documentation comment attached to a given declaration,
   /// without looking into cache.
   RawComment *getRawCommentForDeclNoCache(const Decl *D) const;
@@ -818,6 +796,16 @@ public:
   getRawCommentForAnyRedecl(const Decl *D,
                             const Decl **OriginalDecl = nullptr) const;
 
+  /// Searches existing comments for doc comments that should be attached to \p
+  /// Decls. If any doc comment is found, it is parsed.
+  ///
+  /// Requirement: All \p Decls are in the same file.
+  ///
+  /// If the last comment in the file is already attached we assume
+  /// there are not comments left to be attached to \p Decls.
+  void attachCommentsToJustParsedDecls(ArrayRef<Decl *> Decls,
+                                       const Preprocessor *PP);
+
   /// Return parsed documentation comment attached to a given declaration.
   /// Returns nullptr if no comment is attached.
   ///

Modified: cfe/trunk/include/clang/AST/RawCommentList.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/RawCommentList.h?rev=368732&r1=368731&r2=368732&view=diff
==============================================================================
--- cfe/trunk/include/clang/AST/RawCommentList.h (original)
+++ cfe/trunk/include/clang/AST/RawCommentList.h Tue Aug 13 11:11:44 2019
@@ -10,8 +10,11 @@
 #define LLVM_CLANG_AST_RAWCOMMENTLIST_H
 
 #include "clang/Basic/CommentOptions.h"
+#include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/SourceManager.h"
 #include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/DenseMap.h"
+#include <map>
 
 namespace clang {
 
@@ -196,17 +199,24 @@ public:
   void addComment(const RawComment &RC, const CommentOptions &CommentOpts,
                   llvm::BumpPtrAllocator &Allocator);
 
-  ArrayRef<RawComment *> getComments() const {
-    return Comments;
-  }
+  /// \returns nullptr in case there are no comments in in \p File.
+  const std::map<unsigned, RawComment *> *getCommentsInFile(FileID File) const;
+
+  bool empty() const;
+
+  unsigned getCommentBeginLine(RawComment *C, FileID File,
+                               unsigned Offset) const;
+  unsigned getCommentEndOffset(RawComment *C) const;
 
 private:
   SourceManager &SourceMgr;
-  std::vector<RawComment *> Comments;
-
-  void addDeserializedComments(ArrayRef<RawComment *> DeserializedComments);
+  // mapping: FileId -> comment begin offset -> comment
+  llvm::DenseMap<FileID, std::map<unsigned, RawComment *>> OrderedComments;
+  mutable llvm::DenseMap<RawComment *, unsigned> CommentBeginLine;
+  mutable llvm::DenseMap<RawComment *, unsigned> CommentEndOffset;
 
   friend class ASTReader;
+  friend class ASTWriter;
 };
 
 } // end namespace clang

Modified: cfe/trunk/lib/AST/ASTContext.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTContext.cpp?rev=368732&r1=368731&r2=368732&view=diff
==============================================================================
--- cfe/trunk/lib/AST/ASTContext.cpp (original)
+++ cfe/trunk/lib/AST/ASTContext.cpp Tue Aug 13 11:11:44 2019
@@ -98,62 +98,60 @@ enum FloatingRank {
   Float16Rank, HalfRank, FloatRank, DoubleRank, LongDoubleRank, Float128Rank
 };
 
-RawComment *ASTContext::getRawCommentForDeclNoCache(const Decl *D) const {
+/// \returns location that is relevant when searching for Doc comments related
+/// to \p D.
+static SourceLocation getDeclLocForCommentSearch(const Decl *D,
+                                                 SourceManager &SourceMgr) {
   assert(D);
 
-  // If we already tried to load comments but there are none,
-  // we won't find anything.
-  if (CommentsLoaded && Comments.getComments().empty())
-    return nullptr;
-
   // User can not attach documentation to implicit declarations.
   if (D->isImplicit())
-    return nullptr;
+    return {};
 
   // User can not attach documentation to implicit instantiations.
   if (const auto *FD = dyn_cast<FunctionDecl>(D)) {
     if (FD->getTemplateSpecializationKind() == TSK_ImplicitInstantiation)
-      return nullptr;
+      return {};
   }
 
   if (const auto *VD = dyn_cast<VarDecl>(D)) {
     if (VD->isStaticDataMember() &&
         VD->getTemplateSpecializationKind() == TSK_ImplicitInstantiation)
-      return nullptr;
+      return {};
   }
 
   if (const auto *CRD = dyn_cast<CXXRecordDecl>(D)) {
     if (CRD->getTemplateSpecializationKind() == TSK_ImplicitInstantiation)
-      return nullptr;
+      return {};
   }
 
   if (const auto *CTSD = dyn_cast<ClassTemplateSpecializationDecl>(D)) {
     TemplateSpecializationKind TSK = CTSD->getSpecializationKind();
     if (TSK == TSK_ImplicitInstantiation ||
         TSK == TSK_Undeclared)
-      return nullptr;
+      return {};
   }
 
   if (const auto *ED = dyn_cast<EnumDecl>(D)) {
     if (ED->getTemplateSpecializationKind() == TSK_ImplicitInstantiation)
-      return nullptr;
+      return {};
   }
   if (const auto *TD = dyn_cast<TagDecl>(D)) {
     // When tag declaration (but not definition!) is part of the
     // decl-specifier-seq of some other declaration, it doesn't get comment
     if (TD->isEmbeddedInDeclarator() && !TD->isCompleteDefinition())
-      return nullptr;
+      return {};
   }
   // TODO: handle comments for function parameters properly.
   if (isa<ParmVarDecl>(D))
-    return nullptr;
+    return {};
 
   // TODO: we could look up template parameter documentation in the template
   // documentation.
   if (isa<TemplateTypeParmDecl>(D) ||
       isa<NonTypeTemplateParmDecl>(D) ||
       isa<TemplateTemplateParmDecl>(D))
-    return nullptr;
+    return {};
 
   // Find declaration location.
   // For Objective-C declarations we generally don't expect to have multiple
@@ -161,20 +159,19 @@ RawComment *ASTContext::getRawCommentFor
   // location".
   // For all other declarations multiple declarators are used quite frequently,
   // so we use the location of the identifier as the "declaration location".
-  SourceLocation DeclLoc;
   if (isa<ObjCMethodDecl>(D) || isa<ObjCContainerDecl>(D) ||
       isa<ObjCPropertyDecl>(D) ||
       isa<RedeclarableTemplateDecl>(D) ||
       isa<ClassTemplateSpecializationDecl>(D))
-    DeclLoc = D->getBeginLoc();
+    return D->getBeginLoc();
   else {
-    DeclLoc = D->getLocation();
+    const SourceLocation DeclLoc = D->getLocation();
     if (DeclLoc.isMacroID()) {
       if (isa<TypedefDecl>(D)) {
         // If location of the typedef name is in a macro, it is because being
         // declared via a macro. Try using declaration's starting location as
         // the "declaration location".
-        DeclLoc = D->getBeginLoc();
+        return D->getBeginLoc();
       } else if (const auto *TD = dyn_cast<TagDecl>(D)) {
         // If location of the tag decl is inside a macro, but the spelling of
         // the tag name comes from a macro argument, it looks like a special
@@ -183,102 +180,73 @@ RawComment *ASTContext::getRawCommentFor
         // attach the comment to the tag decl.
         if (SourceMgr.isMacroArgExpansion(DeclLoc) &&
             TD->isCompleteDefinition())
-          DeclLoc = SourceMgr.getExpansionLoc(DeclLoc);
+          return SourceMgr.getExpansionLoc(DeclLoc);
       }
     }
+    return DeclLoc;
   }
 
+  return {};
+}
+
+RawComment *ASTContext::getRawCommentForDeclNoCacheImpl(
+    const Decl *D, const SourceLocation RepresentativeLocForDecl,
+    const std::map<unsigned, RawComment *> &CommentsInTheFile) const {
   // If the declaration doesn't map directly to a location in a file, we
   // can't find the comment.
-  if (DeclLoc.isInvalid() || !DeclLoc.isFileID())
+  if (RepresentativeLocForDecl.isInvalid() ||
+      !RepresentativeLocForDecl.isFileID())
     return nullptr;
 
-  if (!CommentsLoaded && ExternalSource) {
-    ExternalSource->ReadComments();
-
-#ifndef NDEBUG
-    ArrayRef<RawComment *> RawComments = Comments.getComments();
-    assert(std::is_sorted(RawComments.begin(), RawComments.end(),
-                          BeforeThanCompare<RawComment>(SourceMgr)));
-#endif
-
-    CommentsLoaded = true;
-  }
-
-  ArrayRef<RawComment *> RawComments = Comments.getComments();
   // If there are no comments anywhere, we won't find anything.
-  if (RawComments.empty())
+  if (CommentsInTheFile.empty())
     return nullptr;
 
-  // Find the comment that occurs just after this declaration.
-  ArrayRef<RawComment *>::iterator Comment;
-  {
-    // When searching for comments during parsing, the comment we are looking
-    // for is usually among the last two comments we parsed -- check them
-    // first.
-    RawComment CommentAtDeclLoc(
-        SourceMgr, SourceRange(DeclLoc), LangOpts.CommentOpts, false);
-    BeforeThanCompare<RawComment> Compare(SourceMgr);
-    ArrayRef<RawComment *>::iterator MaybeBeforeDecl = RawComments.end() - 1;
-    bool Found = Compare(*MaybeBeforeDecl, &CommentAtDeclLoc);
-    if (!Found && RawComments.size() >= 2) {
-      MaybeBeforeDecl--;
-      Found = Compare(*MaybeBeforeDecl, &CommentAtDeclLoc);
-    }
-
-    if (Found) {
-      Comment = MaybeBeforeDecl + 1;
-      assert(Comment ==
-             llvm::lower_bound(RawComments, &CommentAtDeclLoc, Compare));
-    } else {
-      // Slow path.
-      Comment = llvm::lower_bound(RawComments, &CommentAtDeclLoc, Compare);
-    }
-  }
-
   // Decompose the location for the declaration and find the beginning of the
   // file buffer.
-  std::pair<FileID, unsigned> DeclLocDecomp = SourceMgr.getDecomposedLoc(DeclLoc);
+  const std::pair<FileID, unsigned> DeclLocDecomp =
+      SourceMgr.getDecomposedLoc(RepresentativeLocForDecl);
+
+  // Slow path.
+  auto OffsetCommentBehindDecl =
+      CommentsInTheFile.lower_bound(DeclLocDecomp.second);
 
   // First check whether we have a trailing comment.
-  if (Comment != RawComments.end() &&
-      ((*Comment)->isDocumentation() || LangOpts.CommentOpts.ParseAllComments)
-      && (*Comment)->isTrailingComment() &&
-      (isa<FieldDecl>(D) || isa<EnumConstantDecl>(D) || isa<VarDecl>(D) ||
-       isa<ObjCMethodDecl>(D) || isa<ObjCPropertyDecl>(D))) {
-    std::pair<FileID, unsigned> CommentBeginDecomp
-      = SourceMgr.getDecomposedLoc((*Comment)->getSourceRange().getBegin());
-    // Check that Doxygen trailing comment comes after the declaration, starts
-    // on the same line and in the same file as the declaration.
-    if (DeclLocDecomp.first == CommentBeginDecomp.first &&
-        SourceMgr.getLineNumber(DeclLocDecomp.first, DeclLocDecomp.second)
-          == SourceMgr.getLineNumber(CommentBeginDecomp.first,
-                                     CommentBeginDecomp.second)) {
-      (**Comment).setAttached();
-      return *Comment;
+  if (OffsetCommentBehindDecl != CommentsInTheFile.end()) {
+    RawComment *CommentBehindDecl = OffsetCommentBehindDecl->second;
+    if ((CommentBehindDecl->isDocumentation() ||
+         LangOpts.CommentOpts.ParseAllComments) &&
+        CommentBehindDecl->isTrailingComment() &&
+        (isa<FieldDecl>(D) || isa<EnumConstantDecl>(D) || isa<VarDecl>(D) ||
+         isa<ObjCMethodDecl>(D) || isa<ObjCPropertyDecl>(D))) {
+
+      // Check that Doxygen trailing comment comes after the declaration, starts
+      // on the same line and in the same file as the declaration.
+      if (SourceMgr.getLineNumber(DeclLocDecomp.first, DeclLocDecomp.second) ==
+          Comments.getCommentBeginLine(CommentBehindDecl, DeclLocDecomp.first,
+                                       OffsetCommentBehindDecl->first)) {
+        return CommentBehindDecl;
+      }
     }
   }
 
   // The comment just after the declaration was not a trailing comment.
   // Let's look at the previous comment.
-  if (Comment == RawComments.begin())
+  if (OffsetCommentBehindDecl == CommentsInTheFile.begin())
     return nullptr;
-  --Comment;
+
+  auto OffsetCommentBeforeDecl = --OffsetCommentBehindDecl;
+  RawComment *CommentBeforeDecl = OffsetCommentBeforeDecl->second;
 
   // Check that we actually have a non-member Doxygen comment.
-  if (!((*Comment)->isDocumentation() ||
+  if (!(CommentBeforeDecl->isDocumentation() ||
         LangOpts.CommentOpts.ParseAllComments) ||
-      (*Comment)->isTrailingComment())
+      CommentBeforeDecl->isTrailingComment())
     return nullptr;
 
   // Decompose the end of the comment.
-  std::pair<FileID, unsigned> CommentEndDecomp
-    = SourceMgr.getDecomposedLoc((*Comment)->getSourceRange().getEnd());
-
-  // If the comment and the declaration aren't in the same file, then they
-  // aren't related.
-  if (DeclLocDecomp.first != CommentEndDecomp.first)
-    return nullptr;
+  const unsigned CommentEndOffset =
+      Comments.getCommentEndOffset(CommentBeforeDecl);
 
   // Get the corresponding buffer.
   bool Invalid = false;
@@ -288,26 +256,49 @@ RawComment *ASTContext::getRawCommentFor
     return nullptr;
 
   // Extract text between the comment and declaration.
-  StringRef Text(Buffer + CommentEndDecomp.second,
-                 DeclLocDecomp.second - CommentEndDecomp.second);
+  StringRef Text(Buffer + CommentEndOffset,
+                 DeclLocDecomp.second - CommentEndOffset);
 
   // There should be no other declarations or preprocessor directives between
   // comment and declaration.
   if (Text.find_first_of(";{}#@") != StringRef::npos)
     return nullptr;
 
-  (**Comment).setAttached();
-  return *Comment;
+  return CommentBeforeDecl;
+}
+
+RawComment *ASTContext::getRawCommentForDeclNoCache(const Decl *D) const {
+  const SourceLocation DeclLoc = getDeclLocForCommentSearch(D, SourceMgr);
+
+  // If the declaration doesn't map directly to a location in a file, we
+  // can't find the comment.
+  if (DeclLoc.isInvalid() || !DeclLoc.isFileID())
+    return nullptr;
+
+  if (ExternalSource && !CommentsLoaded) {
+    ExternalSource->ReadComments();
+    CommentsLoaded = true;
+  }
+
+  if (Comments.empty())
+    return nullptr;
+
+  const FileID File = SourceMgr.getDecomposedLoc(DeclLoc).first;
+  const auto CommentsInThisFile = Comments.getCommentsInFile(File);
+  if (!CommentsInThisFile || CommentsInThisFile->empty())
+    return nullptr;
+
+  return getRawCommentForDeclNoCacheImpl(D, DeclLoc, *CommentsInThisFile);
 }
 
 /// If we have a 'templated' declaration for a template, adjust 'D' to
 /// refer to the actual template.
 /// If we have an implicit instantiation, adjust 'D' to refer to template.
-static const Decl *adjustDeclToTemplate(const Decl *D) {
-  if (const auto *FD = dyn_cast<FunctionDecl>(D)) {
+static const Decl &adjustDeclToTemplate(const Decl &D) {
+  if (const auto *FD = dyn_cast<FunctionDecl>(&D)) {
     // Is this function declaration part of a function template?
     if (const FunctionTemplateDecl *FTD = FD->getDescribedFunctionTemplate())
-      return FTD;
+      return *FTD;
 
     // Nothing to do if function is not an implicit instantiation.
     if (FD->getTemplateSpecializationKind() != TSK_ImplicitInstantiation)
@@ -315,28 +306,28 @@ static const Decl *adjustDeclToTemplate(
 
     // Function is an implicit instantiation of a function template?
     if (const FunctionTemplateDecl *FTD = FD->getPrimaryTemplate())
-      return FTD;
+      return *FTD;
 
     // Function is instantiated from a member definition of a class template?
     if (const FunctionDecl *MemberDecl =
             FD->getInstantiatedFromMemberFunction())
-      return MemberDecl;
+      return *MemberDecl;
 
     return D;
   }
-  if (const auto *VD = dyn_cast<VarDecl>(D)) {
+  if (const auto *VD = dyn_cast<VarDecl>(&D)) {
     // Static data member is instantiated from a member definition of a class
     // template?
     if (VD->isStaticDataMember())
       if (const VarDecl *MemberDecl = VD->getInstantiatedFromStaticDataMember())
-        return MemberDecl;
+        return *MemberDecl;
 
     return D;
   }
-  if (const auto *CRD = dyn_cast<CXXRecordDecl>(D)) {
+  if (const auto *CRD = dyn_cast<CXXRecordDecl>(&D)) {
     // Is this class declaration part of a class template?
     if (const ClassTemplateDecl *CTD = CRD->getDescribedClassTemplate())
-      return CTD;
+      return *CTD;
 
     // Class is an implicit instantiation of a class template or partial
     // specialization?
@@ -346,23 +337,23 @@ static const Decl *adjustDeclToTemplate(
       llvm::PointerUnion<ClassTemplateDecl *,
                          ClassTemplatePartialSpecializationDecl *>
           PU = CTSD->getSpecializedTemplateOrPartial();
-      return PU.is<ClassTemplateDecl*>() ?
-          static_cast<const Decl*>(PU.get<ClassTemplateDecl *>()) :
-          static_cast<const Decl*>(
-              PU.get<ClassTemplatePartialSpecializationDecl *>());
+      return PU.is<ClassTemplateDecl *>()
+                 ? *static_cast<const Decl *>(PU.get<ClassTemplateDecl *>())
+                 : *static_cast<const Decl *>(
+                       PU.get<ClassTemplatePartialSpecializationDecl *>());
     }
 
     // Class is instantiated from a member definition of a class template?
     if (const MemberSpecializationInfo *Info =
-                   CRD->getMemberSpecializationInfo())
-      return Info->getInstantiatedFrom();
+            CRD->getMemberSpecializationInfo())
+      return *Info->getInstantiatedFrom();
 
     return D;
   }
-  if (const auto *ED = dyn_cast<EnumDecl>(D)) {
+  if (const auto *ED = dyn_cast<EnumDecl>(&D)) {
     // Enum is instantiated from a member definition of a class template?
     if (const EnumDecl *MemberDecl = ED->getInstantiatedFromMemberEnum())
-      return MemberDecl;
+      return *MemberDecl;
 
     return D;
   }
@@ -373,72 +364,81 @@ static const Decl *adjustDeclToTemplate(
 const RawComment *ASTContext::getRawCommentForAnyRedecl(
                                                 const Decl *D,
                                                 const Decl **OriginalDecl) const {
-  D = adjustDeclToTemplate(D);
+  if (!D) {
+    if (OriginalDecl)
+      OriginalDecl = nullptr;
+    return nullptr;
+  }
 
-  // Check whether we have cached a comment for this declaration already.
+  D = &adjustDeclToTemplate(*D);
+
+  // Any comment directly attached to D?
   {
-    llvm::DenseMap<const Decl *, RawCommentAndCacheFlags>::iterator Pos =
-        RedeclComments.find(D);
-    if (Pos != RedeclComments.end()) {
-      const RawCommentAndCacheFlags &Raw = Pos->second;
-      if (Raw.getKind() != RawCommentAndCacheFlags::NoCommentInDecl) {
-        if (OriginalDecl)
-          *OriginalDecl = Raw.getOriginalDecl();
-        return Raw.getRaw();
-      }
+    auto DeclComment = DeclRawComments.find(D);
+    if (DeclComment != DeclRawComments.end()) {
+      if (OriginalDecl)
+        *OriginalDecl = D;
+      return DeclComment->second;
     }
   }
 
-  // Search for comments attached to declarations in the redeclaration chain.
-  const RawComment *RC = nullptr;
-  const Decl *OriginalDeclForRC = nullptr;
-  for (auto I : D->redecls()) {
-    llvm::DenseMap<const Decl *, RawCommentAndCacheFlags>::iterator Pos =
-        RedeclComments.find(I);
-    if (Pos != RedeclComments.end()) {
-      const RawCommentAndCacheFlags &Raw = Pos->second;
-      if (Raw.getKind() != RawCommentAndCacheFlags::NoCommentInDecl) {
-        RC = Raw.getRaw();
-        OriginalDeclForRC = Raw.getOriginalDecl();
-        break;
-      }
-    } else {
-      RC = getRawCommentForDeclNoCache(I);
-      OriginalDeclForRC = I;
-      RawCommentAndCacheFlags Raw;
-      if (RC) {
-        // Call order swapped to work around ICE in VS2015 RTM (Release Win32)
-        // https://connect.microsoft.com/VisualStudio/feedback/details/1741530
-        Raw.setKind(RawCommentAndCacheFlags::FromDecl);
-        Raw.setRaw(RC);
-      } else
-        Raw.setKind(RawCommentAndCacheFlags::NoCommentInDecl);
-      Raw.setOriginalDecl(I);
-      RedeclComments[I] = Raw;
-      if (RC)
-        break;
+  // Any comment attached to any redeclaration of D?
+  const Decl *CanonicalD = D->getCanonicalDecl();
+  if (!CanonicalD)
+    return nullptr;
+
+  {
+    auto RedeclComment = RedeclChainComments.find(CanonicalD);
+    if (RedeclComment != RedeclChainComments.end()) {
+      if (OriginalDecl)
+        *OriginalDecl = RedeclComment->second;
+      auto CommentAtRedecl = DeclRawComments.find(RedeclComment->second);
+      assert(CommentAtRedecl != DeclRawComments.end() &&
+             "This decl is supposed to have comment attached.");
+      return CommentAtRedecl->second;
     }
   }
 
-  // If we found a comment, it should be a documentation comment.
-  assert(!RC || RC->isDocumentation() || LangOpts.CommentOpts.ParseAllComments);
+  // Any redeclarations of D that we haven't checked for comments yet?
+  // We can't use DenseMap::iterator directly since it'd get invalid.
+  auto LastCheckedRedecl = [this, CanonicalD]() -> const Decl * {
+    auto LookupRes = CommentlessRedeclChains.find(CanonicalD);
+    if (LookupRes != CommentlessRedeclChains.end())
+      return LookupRes->second;
+    return nullptr;
+  }();
+
+  for (const auto Redecl : D->redecls()) {
+    assert(Redecl);
+    // Skip all redeclarations that have been checked previously.
+    if (LastCheckedRedecl) {
+      if (LastCheckedRedecl == Redecl) {
+        LastCheckedRedecl = nullptr;
+      }
+      continue;
+    }
+    const RawComment *RedeclComment = getRawCommentForDeclNoCache(Redecl);
+    if (RedeclComment) {
+      cacheRawCommentForDecl(*Redecl, *RedeclComment);
+      if (OriginalDecl)
+        *OriginalDecl = Redecl;
+      return RedeclComment;
+    }
+    CommentlessRedeclChains[CanonicalD] = Redecl;
+  }
 
   if (OriginalDecl)
-    *OriginalDecl = OriginalDeclForRC;
-
-  // Update cache for every declaration in the redeclaration chain.
-  RawCommentAndCacheFlags Raw;
-  Raw.setRaw(RC);
-  Raw.setKind(RawCommentAndCacheFlags::FromRedecl);
-  Raw.setOriginalDecl(OriginalDeclForRC);
-
-  for (auto I : D->redecls()) {
-    RawCommentAndCacheFlags &R = RedeclComments[I];
-    if (R.getKind() == RawCommentAndCacheFlags::NoCommentInDecl)
-      R = Raw;
-  }
+    *OriginalDecl = nullptr;
+  return nullptr;
+}
 
-  return RC;
+void ASTContext::cacheRawCommentForDecl(const Decl &OriginalD,
+                                        const RawComment &Comment) const {
+  assert(Comment.isDocumentation() || LangOpts.CommentOpts.ParseAllComments);
+  DeclRawComments.try_emplace(&OriginalD, &Comment);
+  const Decl *const CanonicalDecl = OriginalD.getCanonicalDecl();
+  RedeclChainComments.try_emplace(CanonicalDecl, &OriginalD);
+  CommentlessRedeclChains.erase(CanonicalDecl);
 }
 
 static void addRedeclaredMethods(const ObjCMethodDecl *ObjCMethod,
@@ -458,6 +458,52 @@ static void addRedeclaredMethods(const O
   }
 }
 
+void ASTContext::attachCommentsToJustParsedDecls(ArrayRef<Decl *> Decls,
+                                                 const Preprocessor *PP) {
+  if (Comments.empty() || Decls.empty())
+    return;
+
+  // See if there are any new comments that are not attached to a decl.
+  // The location doesn't have to be precise - we care only about the file.
+  const FileID File =
+      SourceMgr.getDecomposedLoc((*Decls.begin())->getLocation()).first;
+  auto CommentsInThisFile = Comments.getCommentsInFile(File);
+  if (!CommentsInThisFile || CommentsInThisFile->empty() ||
+      CommentsInThisFile->rbegin()->second->isAttached())
+    return;
+
+  // There is at least one comment not attached to a decl.
+  // Maybe it should be attached to one of Decls?
+  //
+  // Note that this way we pick up not only comments that precede the
+  // declaration, but also comments that *follow* the declaration -- thanks to
+  // the lookahead in the lexer: we've consumed the semicolon and looked
+  // ahead through comments.
+
+  for (const Decl *D : Decls) {
+    assert(D);
+    if (D->isInvalidDecl())
+      continue;
+
+    D = &adjustDeclToTemplate(*D);
+
+    const SourceLocation DeclLoc = getDeclLocForCommentSearch(D, SourceMgr);
+
+    if (DeclLoc.isInvalid() || !DeclLoc.isFileID())
+      continue;
+
+    if (DeclRawComments.count(D) > 0)
+      continue;
+
+    if (RawComment *const DocComment =
+            getRawCommentForDeclNoCacheImpl(D, DeclLoc, *CommentsInThisFile)) {
+      cacheRawCommentForDecl(*D, *DocComment);
+      comments::FullComment *FC = DocComment->parse(*this, PP, D);
+      ParsedComments[D->getCanonicalDecl()] = FC;
+    }
+  }
+}
+
 comments::FullComment *ASTContext::cloneFullComment(comments::FullComment *FC,
                                                     const Decl *D) const {
   auto *ThisDeclInfo = new (*this) comments::DeclInfo;
@@ -481,9 +527,9 @@ comments::FullComment *ASTContext::getLo
 comments::FullComment *ASTContext::getCommentForDecl(
                                               const Decl *D,
                                               const Preprocessor *PP) const {
-  if (D->isInvalidDecl())
+  if (!D || D->isInvalidDecl())
     return nullptr;
-  D = adjustDeclToTemplate(D);
+  D = &adjustDeclToTemplate(*D);
 
   const Decl *Canonical = D->getCanonicalDecl();
   llvm::DenseMap<const Decl *, comments::FullComment *>::iterator Pos =
@@ -498,7 +544,7 @@ comments::FullComment *ASTContext::getCo
     return Pos->second;
   }
 
-  const Decl *OriginalDecl;
+  const Decl *OriginalDecl = nullptr;
 
   const RawComment *RC = getRawCommentForAnyRedecl(D, &OriginalDecl);
   if (!RC) {
@@ -577,7 +623,7 @@ comments::FullComment *ASTContext::getCo
   // should parse the comment in context of that other Decl.  This is important
   // because comments can contain references to parameter names which can be
   // different across redeclarations.
-  if (D != OriginalDecl)
+  if (D != OriginalDecl && OriginalDecl)
     return getCommentForDecl(OriginalDecl, PP);
 
   comments::FullComment *FC = RC->parse(*this, PP, D);

Modified: cfe/trunk/lib/AST/RawCommentList.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/RawCommentList.cpp?rev=368732&r1=368731&r2=368732&view=diff
==============================================================================
--- cfe/trunk/lib/AST/RawCommentList.cpp (original)
+++ cfe/trunk/lib/AST/RawCommentList.cpp Tue Aug 13 11:11:44 2019
@@ -275,27 +275,25 @@ void RawCommentList::addComment(const Ra
   if (RC.isInvalid())
     return;
 
-  // Check if the comments are not in source order.
-  while (!Comments.empty() &&
-         !SourceMgr.isBeforeInTranslationUnit(Comments.back()->getBeginLoc(),
-                                              RC.getBeginLoc())) {
-    // If they are, just pop a few last comments that don't fit.
-    // This happens if an \#include directive contains comments.
-    Comments.pop_back();
-  }
-
   // Ordinary comments are not interesting for us.
   if (RC.isOrdinary() && !CommentOpts.ParseAllComments)
     return;
 
+  std::pair<FileID, unsigned> Loc =
+      SourceMgr.getDecomposedLoc(RC.getBeginLoc());
+
+  const FileID CommentFile = Loc.first;
+  const unsigned CommentOffset = Loc.second;
+
   // If this is the first Doxygen comment, save it (because there isn't
   // anything to merge it with).
-  if (Comments.empty()) {
-    Comments.push_back(new (Allocator) RawComment(RC));
+  if (OrderedComments[CommentFile].empty()) {
+    OrderedComments[CommentFile][CommentOffset] =
+        new (Allocator) RawComment(RC);
     return;
   }
 
-  const RawComment &C1 = *Comments.back();
+  const RawComment &C1 = *OrderedComments[CommentFile].rbegin()->second;
   const RawComment &C2 = RC;
 
   // Merge comments only if there is only whitespace between them.
@@ -318,21 +316,43 @@ void RawCommentList::addComment(const Ra
       onlyWhitespaceBetween(SourceMgr, C1.getEndLoc(), C2.getBeginLoc(),
                             /*MaxNewlinesAllowed=*/1)) {
     SourceRange MergedRange(C1.getBeginLoc(), C2.getEndLoc());
-    *Comments.back() = RawComment(SourceMgr, MergedRange, CommentOpts, true);
+    *OrderedComments[CommentFile].rbegin()->second =
+        RawComment(SourceMgr, MergedRange, CommentOpts, true);
   } else {
-    Comments.push_back(new (Allocator) RawComment(RC));
+    OrderedComments[CommentFile][CommentOffset] =
+        new (Allocator) RawComment(RC);
   }
 }
 
-void RawCommentList::addDeserializedComments(ArrayRef<RawComment *> DeserializedComments) {
-  std::vector<RawComment *> MergedComments;
-  MergedComments.reserve(Comments.size() + DeserializedComments.size());
-
-  std::merge(Comments.begin(), Comments.end(),
-             DeserializedComments.begin(), DeserializedComments.end(),
-             std::back_inserter(MergedComments),
-             BeforeThanCompare<RawComment>(SourceMgr));
-  std::swap(Comments, MergedComments);
+const std::map<unsigned, RawComment *> *
+RawCommentList::getCommentsInFile(FileID File) const {
+  auto CommentsInFile = OrderedComments.find(File);
+  if (CommentsInFile == OrderedComments.end())
+    return nullptr;
+
+  return &CommentsInFile->second;
+}
+
+bool RawCommentList::empty() const { return OrderedComments.empty(); }
+
+unsigned RawCommentList::getCommentBeginLine(RawComment *C, FileID File,
+                                             unsigned Offset) const {
+  auto Cached = CommentBeginLine.find(C);
+  if (Cached != CommentBeginLine.end())
+    return Cached->second;
+  const unsigned Line = SourceMgr.getLineNumber(File, Offset);
+  CommentBeginLine[C] = Line;
+  return Line;
+}
+
+unsigned RawCommentList::getCommentEndOffset(RawComment *C) const {
+  auto Cached = CommentEndOffset.find(C);
+  if (Cached != CommentEndOffset.end())
+    return Cached->second;
+  const unsigned Offset =
+      SourceMgr.getDecomposedLoc(C->getSourceRange().getEnd()).second;
+  CommentEndOffset[C] = Offset;
+  return Offset;
 }
 
 std::string RawComment::getFormattedText(const SourceManager &SourceMgr,

Modified: cfe/trunk/lib/Sema/SemaDecl.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDecl.cpp?rev=368732&r1=368731&r2=368732&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaDecl.cpp (original)
+++ cfe/trunk/lib/Sema/SemaDecl.cpp Tue Aug 13 11:11:44 2019
@@ -12465,20 +12465,10 @@ void Sema::ActOnDocumentableDecls(ArrayR
     }
   }
 
-  // See if there are any new comments that are not attached to a decl.
-  ArrayRef<RawComment *> Comments = Context.getRawCommentList().getComments();
-  if (!Comments.empty() &&
-      !Comments.back()->isAttached()) {
-    // There is at least one comment that not attached to a decl.
-    // Maybe it should be attached to one of these decls?
-    //
-    // Note that this way we pick up not only comments that precede the
-    // declaration, but also comments that *follow* the declaration -- thanks to
-    // the lookahead in the lexer: we've consumed the semicolon and looked
-    // ahead through comments.
-    for (unsigned i = 0, e = Group.size(); i != e; ++i)
-      Context.getCommentForDecl(Group[i], &PP);
-  }
+  // FIMXE: We assume every Decl in the group is in the same file.
+  // This is false when preprocessor constructs the group from decls in
+  // different files (e. g. macros or #include).
+  Context.attachCommentsToJustParsedDecls(Group, &getPreprocessor());
 }
 
 /// Common checks for a parameter-declaration that should apply to both function

Modified: cfe/trunk/lib/Serialization/ASTReader.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTReader.cpp?rev=368732&r1=368731&r2=368732&view=diff
==============================================================================
--- cfe/trunk/lib/Serialization/ASTReader.cpp (original)
+++ cfe/trunk/lib/Serialization/ASTReader.cpp Tue Aug 13 11:11:44 2019
@@ -9731,10 +9731,17 @@ void ASTReader::ReadComments() {
       }
     }
   NextCursor:
-    // De-serialized SourceLocations get negative FileIDs for other modules,
-    // potentially invalidating the original order. Sort it again.
-    llvm::sort(Comments, BeforeThanCompare<RawComment>(SourceMgr));
-    Context.Comments.addDeserializedComments(Comments);
+    llvm::DenseMap<FileID, std::map<unsigned, RawComment *>>
+        FileToOffsetToComment;
+    for (RawComment *C : Comments) {
+      SourceLocation CommentLoc = C->getBeginLoc();
+      if (CommentLoc.isValid()) {
+        std::pair<FileID, unsigned> Loc =
+            SourceMgr.getDecomposedLoc(CommentLoc);
+        if (Loc.first.isValid())
+          Context.Comments.OrderedComments[Loc.first].emplace(Loc.second, C);
+      }
+    }
   }
 }
 

Modified: cfe/trunk/lib/Serialization/ASTWriter.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTWriter.cpp?rev=368732&r1=368731&r2=368732&view=diff
==============================================================================
--- cfe/trunk/lib/Serialization/ASTWriter.cpp (original)
+++ cfe/trunk/lib/Serialization/ASTWriter.cpp Tue Aug 13 11:11:44 2019
@@ -3266,15 +3266,17 @@ void ASTWriter::WriteComments() {
   auto _ = llvm::make_scope_exit([this] { Stream.ExitBlock(); });
   if (!PP->getPreprocessorOpts().WriteCommentListToPCH)
     return;
-  ArrayRef<RawComment *> RawComments = Context->Comments.getComments();
   RecordData Record;
-  for (const auto *I : RawComments) {
-    Record.clear();
-    AddSourceRange(I->getSourceRange(), Record);
-    Record.push_back(I->getKind());
-    Record.push_back(I->isTrailingComment());
-    Record.push_back(I->isAlmostTrailingComment());
-    Stream.EmitRecord(COMMENTS_RAW_COMMENT, Record);
+  for (const auto &FO : Context->Comments.OrderedComments) {
+    for (const auto &OC : FO.second) {
+      const RawComment *I = OC.second;
+      Record.clear();
+      AddSourceRange(I->getSourceRange(), Record);
+      Record.push_back(I->getKind());
+      Record.push_back(I->isTrailingComment());
+      Record.push_back(I->isAlmostTrailingComment());
+      Stream.EmitRecord(COMMENTS_RAW_COMMENT, Record);
+    }
   }
 }
 

Added: cfe/trunk/test/Index/comment-redeclarations.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Index/comment-redeclarations.cpp?rev=368732&view=auto
==============================================================================
--- cfe/trunk/test/Index/comment-redeclarations.cpp (added)
+++ cfe/trunk/test/Index/comment-redeclarations.cpp Tue Aug 13 11:11:44 2019
@@ -0,0 +1,14 @@
+// RUN: rm -rf %t
+// RUN: mkdir %t
+// RUN: c-index-test -test-load-source all -comments-xml-schema=%S/../../bindings/xml/comment-xml-schema.rng -target x86_64-apple-darwin10 %s > %t/out
+// RUN: FileCheck %s < %t/out
+
+class Foo;
+// CHECK: CXComment_Text Text=[ Foo is the best!])))]
+
+/// Foo is the best!
+class Foo;
+// CHECK: CXComment_Text Text=[ Foo is the best!])))]
+
+class Foo {};
+// CHECK: CXComment_Text Text=[ Foo is the best!])))]




More information about the cfe-commits mailing list