[clang][ASTContext][NFCi] Refactor ASTContext::getRawCommentForDeclNoCache

[jkorous]

Summary

Factor out utility functions for documentation comments to be used in upcoming patches.

• https://reviews.llvm.org/D61103
• https://reviews.llvm.org/D61104

Also try not to get decomposed source location unless necessary as it is expensive.

[gribozavr]

Could you reupload this patch and other patches with context? It is difficult to review.

clang/lib/AST/ASTContext.cpp

141

No need for a semicolon.

143

Returns true if the comment which ends at CommentEndLoc is attached to the declaration at DeclLoc. The comment must be the closest one before the declaration (no other comments in between).

Also, rename to IsCommentAttachedToDecl?

154

assert?

175

No need for a semicolon.

250

It is really the "declaration location". The following code creates a RawComment with this location just because BeforeThanCompare<RawComment> needs one.

[jkorous]

• renames
• if => assert

[jkorous]

clang/lib/AST/ASTContext.cpp

141

Right, seems like a remnant from when this function was still just a lambda.

250

You're right.

[jkorous]

• moved assert

[jkorous]

Abandoned in favor of
Differential Revision: https://reviews.llvm.org/D65301

clang/lib/AST/ASTContext.cpp

   Float16Rank, HalfRank, FloatRank, DoubleRank, LongDoubleRank, Float128Rank
 };
 
-RawComment *ASTContext::getRawCommentForDeclNoCache(const Decl *D) const {
-  assert(D);
+static llvm::Optional<SourceLocation>
+getCandidateCommentLocation(SourceManager &SrcMgr, const Decl *D) {
+  // Find declaration location.
+  // For Objective-C declarations we generally don't expect to have multiple
+  // declarators, thus use declaration starting location as the "declaration
+  // 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();
+  else {
+    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();
+      } 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
+        // macro like NS_ENUM is being used to define the tag decl.  In that
+        // case, adjust the source location to the expansion loc so that we can
+        // attach the comment to the tag decl.
+        if (SrcMgr.isMacroArgExpansion(DeclLoc) && TD->isCompleteDefinition())
+          DeclLoc = SrcMgr.getExpansionLoc(DeclLoc);
+      }
+    }
+  }
 
-  // If we already tried to load comments but there are none,
-  // we won't find anything.
-  if (CommentsLoaded && Comments.getComments().empty())
-    return nullptr;
+  // 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 llvm::None;
+  }
+
+  return DeclLoc;
+};
+
+// Assumes the comment is the closest comment before the decl - i. e. there's no
+// other comment between them.
+static bool IsCommentBeforeDecl(SourceManager &SrcMgr,
+                                std::pair<FileID, unsigned> CommentEndLoc,
+                                std::pair<FileID, unsigned> DeclLoc) {
+  // If the comment and the declaration aren't in the same file, then they
+  // aren't related.
+  if (CommentEndLoc.first != DeclLoc.first)
+    return false;
+
+  // We're looking for comments that are BEFORE this declaration
+  if (CommentEndLoc.second > DeclLoc.second)
+    return false;
+
+  // Get the corresponding buffer.
+  bool Invalid = false;
+  const char *Buffer = SrcMgr.getBufferData(DeclLoc.first, &Invalid).data();
+  if (Invalid)
+    return false;
+
+  // Extract text between the comment and declaration.
+  StringRef Text(Buffer + CommentEndLoc.second,
+                 DeclLoc.second - CommentEndLoc.second);
+
+  // FIXME: This has an assumption that there's no other comment in between
+  // which was always true before the refactoring.
+  // There should be no other declarations or preprocessor directives between
+  // comment and declaration.
+  if (Text.find_first_of(";{}#@") != StringRef::npos)
+    return false;
+
+  return true;
+};
+
+/// Returns true iff \param D can have a documentation comment attached.
+static bool CanDeclHaveDocComment(const Decl *D) {
+  if (!D)
+    return false;
 
   // User can not attach documentation to implicit declarations.
   if (D->isImplicit())
-    return nullptr;
+    return false;
 
   // User can not attach documentation to implicit instantiations.
   if (const auto *FD = dyn_cast<FunctionDecl>(D)) {
     if (FD->getTemplateSpecializationKind() == TSK_ImplicitInstantiation)
-      return nullptr;
+      return false;
   }
 
   if (const auto *VD = dyn_cast<VarDecl>(D)) {
     if (VD->isStaticDataMember() &&
         VD->getTemplateSpecializationKind() == TSK_ImplicitInstantiation)
-      return nullptr;
+      return false;
   }
 
   if (const auto *CRD = dyn_cast<CXXRecordDecl>(D)) {
     if (CRD->getTemplateSpecializationKind() == TSK_ImplicitInstantiation)
-      return nullptr;
+      return false;
   }
 
   if (const auto *CTSD = dyn_cast<ClassTemplateSpecializationDecl>(D)) {
     TemplateSpecializationKind TSK = CTSD->getSpecializationKind();
     if (TSK == TSK_ImplicitInstantiation ||
         TSK == TSK_Undeclared)
-      return nullptr;
+      return false;
   }
 
   if (const auto *ED = dyn_cast<EnumDecl>(D)) {
     if (ED->getTemplateSpecializationKind() == TSK_ImplicitInstantiation)
-      return nullptr;
+      return false;
   }
   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 false;
   }
   // TODO: handle comments for function parameters properly.
   if (isa<ParmVarDecl>(D))
-    return nullptr;
+    return false;
 
   // TODO: we could look up template parameter documentation in the template
   // documentation.
   if (isa<TemplateTypeParmDecl>(D) ||
       isa<NonTypeTemplateParmDecl>(D) ||
       isa<TemplateTemplateParmDecl>(D))
+    return false;
+
+  return true;
+}
+
+RawComment *ASTContext::getRawCommentForDeclNoCache(const Decl *D) const {
+  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;
 
-  // Find declaration location.
-  // For Objective-C declarations we generally don't expect to have multiple
-  // declarators, thus use declaration starting location as the "declaration
-  // 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();
-  else {
-    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();
-      } 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
-        // macro like NS_ENUM is being used to define the tag decl.  In that
-        // case, adjust the source location to the expansion loc so that we can
-        // attach the comment to the tag decl.
-        if (SourceMgr.isMacroArgExpansion(DeclLoc) &&
-            TD->isCompleteDefinition())
-          DeclLoc = SourceMgr.getExpansionLoc(DeclLoc);
-      }
-    }
-  }
+  if (!CanDeclHaveDocComment(D))
+    return nullptr;
 
-  // If the declaration doesn't map directly to a location in a file, we
-  // can't find the comment.
-  if (DeclLoc.isInvalid() || !DeclLoc.isFileID())
+  const llvm::Optional<SourceLocation> OptCandidateCommentLocation =
+      getCandidateCommentLocation(SourceMgr, D);
+  if (!OptCandidateCommentLocation)
     return nullptr;
 
+  const SourceLocation &CandidateCommentLocation =
+      OptCandidateCommentLocation.getValue();
+
   if (!CommentsLoaded && ExternalSource) {
     ExternalSource->ReadComments();
 

   }
 
   ArrayRef<RawComment *> RawComments = Comments.getComments();
+
   // If there are no comments anywhere, we won't find anything.
   if (RawComments.empty())
     return nullptr;

     // 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);
+    RawComment CommentAtDeclLoc(SourceMgr,
+                                SourceRange(CandidateCommentLocation),
+                                LangOpts.CommentOpts, false);
     BeforeThanCompare<RawComment> Compare(SourceMgr);
     ArrayRef<RawComment *>::iterator MaybeBeforeDecl = RawComments.end() - 1;
     bool Found = Compare(*MaybeBeforeDecl, &CommentAtDeclLoc);

     }
   }
 
-  // Decompose the location for the declaration and find the beginning of the
-  // file buffer.
-  std::pair<FileID, unsigned> DeclLocDecomp = SourceMgr.getDecomposedLoc(DeclLoc);
+  // Decomposing SourceLocation is expensive - try to avoid it and/or reuse it.
+  llvm::Optional<std::pair<FileID, unsigned>> OptDeclLocDecomp;
 
   // First check whether we have a trailing comment.
   if (Comment != RawComments.end() &&

       && (*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());
+
+    OptDeclLocDecomp = SourceMgr.getDecomposedLoc(CandidateCommentLocation);
+    const std::pair<FileID, unsigned> &DeclLocDecomp =
+        OptDeclLocDecomp.getValue();
+
+    const 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 &&

       (*Comment)->isTrailingComment())
     return nullptr;
 
-  // Decompose the end of the comment.
+  if (!OptDeclLocDecomp)
+    OptDeclLocDecomp = SourceMgr.getDecomposedLoc(CandidateCommentLocation);
+  const std::pair<FileID, unsigned> &DeclLocDecomp =
+      OptDeclLocDecomp.getValue();
+
   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;
-
-  // Get the corresponding buffer.
-  bool Invalid = false;
-  const char *Buffer = SourceMgr.getBufferData(DeclLocDecomp.first,
-                                               &Invalid).data();
-  if (Invalid)
-    return nullptr;
-
-  // Extract text between the comment and declaration.
-  StringRef Text(Buffer + CommentEndDecomp.second,
-                 DeclLocDecomp.second - CommentEndDecomp.second);
-
-  // There should be no other declarations or preprocessor directives between
-  // comment and declaration.
-  if (Text.find_first_of(";{}#@") != StringRef::npos)
+  if (!IsCommentBeforeDecl(SourceMgr, CommentEndDecomp, DeclLocDecomp))
     return nullptr;
 
   return *Comment;

clang/lib/AST/RawCommentList.cpp

 }
 
 static bool onlyWhitespaceBetween(SourceManager &SM,
-                                  SourceLocation Loc1, SourceLocation Loc2,
+                                  const std::pair<FileID, unsigned> &Loc1Info,
+                                  const std::pair<FileID, unsigned> &Loc2Info,
                                   unsigned MaxNewlinesAllowed) {
-  std::pair<FileID, unsigned> Loc1Info = SM.getDecomposedLoc(Loc1);
-  std::pair<FileID, unsigned> Loc2Info = SM.getDecomposedLoc(Loc2);
-
   // Question does not make sense if locations are in different files.
   if (Loc1Info.first != Loc2Info.first)
     return false;

        (C1.isTrailingComment() && !C2.isTrailingComment() &&
         isOrdinaryKind(C2.getKind()) &&
         commentsStartOnSameColumn(SourceMgr, C1, C2))) &&
-      onlyWhitespaceBetween(SourceMgr, C1.getEndLoc(), C2.getBeginLoc(),
+      onlyWhitespaceBetween(SourceMgr,
+                            SourceMgr.getDecomposedLoc(C1.getEndLoc()),
+                            SourceMgr.getDecomposedLoc(C2.getBeginLoc()),
                             /*MaxNewlinesAllowed=*/1)) {
     SourceRange MergedRange(C1.getBeginLoc(), C2.getEndLoc());
     *Comments.back() = RawComment(SourceMgr, MergedRange, CommentOpts, true);