Index: clang/lib/AST/ASTContext.cpp =================================================================== --- clang/lib/AST/ASTContext.cpp +++ clang/lib/AST/ASTContext.cpp @@ -98,101 +98,158 @@ Float16Rank, HalfRank, FloatRank, DoubleRank, LongDoubleRank, Float128Rank }; -RawComment *ASTContext::getRawCommentForDeclNoCache(const Decl *D) const { - assert(D); +static llvm::Optional +getDeclLocationForCommentSearch(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(D) || isa(D) || + isa(D) || isa(D) || + isa(D)) + DeclLoc = D->getBeginLoc(); + else { + DeclLoc = D->getLocation(); + if (DeclLoc.isMacroID()) { + if (isa(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(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; +}; + +// 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). +static bool IsCommentAttachedToDecl(SourceManager &SrcMgr, + std::pair CommentEndLoc, + std::pair 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 + assert(CommentEndLoc.second < DeclLoc.second); + + // 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(D)) { if (FD->getTemplateSpecializationKind() == TSK_ImplicitInstantiation) - return nullptr; + return false; } if (const auto *VD = dyn_cast(D)) { if (VD->isStaticDataMember() && VD->getTemplateSpecializationKind() == TSK_ImplicitInstantiation) - return nullptr; + return false; } if (const auto *CRD = dyn_cast(D)) { if (CRD->getTemplateSpecializationKind() == TSK_ImplicitInstantiation) - return nullptr; + return false; } if (const auto *CTSD = dyn_cast(D)) { TemplateSpecializationKind TSK = CTSD->getSpecializationKind(); if (TSK == TSK_ImplicitInstantiation || TSK == TSK_Undeclared) - return nullptr; + return false; } if (const auto *ED = dyn_cast(D)) { if (ED->getTemplateSpecializationKind() == TSK_ImplicitInstantiation) - return nullptr; + return false; } if (const auto *TD = dyn_cast(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(D)) - return nullptr; + return false; // TODO: we could look up template parameter documentation in the template // documentation. if (isa(D) || isa(D) || isa(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(D) || isa(D) || - isa(D) || - isa(D) || - isa(D)) - DeclLoc = D->getBeginLoc(); - else { - DeclLoc = D->getLocation(); - if (DeclLoc.isMacroID()) { - if (isa(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(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 OptDeclLocForCommentSearch = + getDeclLocationForCommentSearch(SourceMgr, D); + if (!OptDeclLocForCommentSearch) return nullptr; + const SourceLocation &DeclLocForCommentSearch = + OptDeclLocForCommentSearch.getValue(); + if (!CommentsLoaded && ExternalSource) { ExternalSource->ReadComments(); @@ -206,6 +263,7 @@ } ArrayRef RawComments = Comments.getComments(); + // If there are no comments anywhere, we won't find anything. if (RawComments.empty()) return nullptr; @@ -216,8 +274,8 @@ // 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(DeclLocForCommentSearch), + LangOpts.CommentOpts, false); BeforeThanCompare Compare(SourceMgr); ArrayRef::iterator MaybeBeforeDecl = RawComments.end() - 1; bool Found = Compare(*MaybeBeforeDecl, &CommentAtDeclLoc); @@ -237,9 +295,8 @@ } } - // Decompose the location for the declaration and find the beginning of the - // file buffer. - std::pair DeclLocDecomp = SourceMgr.getDecomposedLoc(DeclLoc); + // Decomposing SourceLocation is expensive - try to avoid it and/or reuse it. + llvm::Optional> OptDeclLocDecomp; // First check whether we have a trailing comment. if (Comment != RawComments.end() && @@ -247,8 +304,13 @@ && (*Comment)->isTrailingComment() && (isa(D) || isa(D) || isa(D) || isa(D) || isa(D))) { - std::pair CommentBeginDecomp - = SourceMgr.getDecomposedLoc((*Comment)->getSourceRange().getBegin()); + + OptDeclLocDecomp = SourceMgr.getDecomposedLoc(DeclLocForCommentSearch); + const std::pair &DeclLocDecomp = + OptDeclLocDecomp.getValue(); + + const std::pair 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 && @@ -271,29 +333,15 @@ (*Comment)->isTrailingComment()) return nullptr; - // Decompose the end of the comment. + if (!OptDeclLocDecomp) + OptDeclLocDecomp = SourceMgr.getDecomposedLoc(DeclLocForCommentSearch); + const std::pair &DeclLocDecomp = + OptDeclLocDecomp.getValue(); + std::pair 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 (!IsCommentAttachedToDecl(SourceMgr, CommentEndDecomp, DeclLocDecomp)) return nullptr; return *Comment; Index: clang/lib/AST/RawCommentList.cpp =================================================================== --- clang/lib/AST/RawCommentList.cpp +++ clang/lib/AST/RawCommentList.cpp @@ -222,11 +222,9 @@ } static bool onlyWhitespaceBetween(SourceManager &SM, - SourceLocation Loc1, SourceLocation Loc2, + const std::pair &Loc1Info, + const std::pair &Loc2Info, unsigned MaxNewlinesAllowed) { - std::pair Loc1Info = SM.getDecomposedLoc(Loc1); - std::pair Loc2Info = SM.getDecomposedLoc(Loc2); - // Question does not make sense if locations are in different files. if (Loc1Info.first != Loc2Info.first) return false; @@ -315,7 +313,9 @@ (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);