diff --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp index 0b6ed82cc5ba0..fea7f2400b31e 100644 --- a/clang/lib/AST/ASTContext.cpp +++ b/clang/lib/AST/ASTContext.cpp @@ -112,10 +112,10 @@ enum FloatingRank { Ibm128Rank }; -/// \returns location that is relevant when searching for Doc comments related -/// to \p D. -static SourceLocation getDeclLocForCommentSearch(const Decl *D, - SourceManager &SourceMgr) { +/// \returns The locations that are relevant when searching for Doc comments +/// related to \p D. +static SmallVector +getDeclLocsForCommentSearch(const Decl *D, SourceManager &SourceMgr) { assert(D); // User can not attach documentation to implicit declarations. @@ -167,115 +167,48 @@ static SourceLocation getDeclLocForCommentSearch(const Decl *D, isa(D)) return {}; + SmallVector Locations; // 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 BaseLocation; if (isa(D) || isa(D) || - isa(D) || - isa(D) || + isa(D) || isa(D) || isa(D) || // Allow association with Y across {} in `typedef struct X {} Y`. isa(D)) - return D->getBeginLoc(); + BaseLocation = D->getBeginLoc(); + else + BaseLocation = D->getLocation(); - const SourceLocation DeclLoc = D->getLocation(); - if (DeclLoc.isMacroID()) { - // There are (at least) three types of macros we care about here. - // - // 1. Macros that are used in the definition of a type outside the macro, - // with a comment attached at the macro call site. - // ``` - // #define MAKE_NAME(Foo) Name##Foo - // - // /// Comment is here, where we use the macro. - // struct MAKE_NAME(Foo) { - // int a; - // int b; - // }; - // ``` - // 2. Macros that define whole things along with the comment. - // ``` - // #define MAKE_METHOD(name) \ - // /** Comment is here, inside the macro. */ \ - // void name() {} - // - // struct S { - // MAKE_METHOD(f) - // } - // ``` - // 3. Macros that both declare a type and name a decl outside the macro. - // ``` - // /// Comment is here, where we use the macro. - // typedef NS_ENUM(NSInteger, Size) { - // SizeWidth, - // SizeHeight - // }; - // ``` - // In this case NS_ENUM declares am enum type, and uses the same name for - // the typedef declaration that appears outside the macro. The comment - // here should be applied to both declarations inside and outside the - // macro. - // - // We have found a Decl name that comes from inside a macro, but - // Decl::getLocation() returns the place where the macro is being called. - // If the declaration (and not just the name) resides inside the macro, - // then we want to map Decl::getLocation() into the macro to where the - // declaration and its attached comment (if any) were written. - // - // This mapping into the macro is done by mapping the location to its - // spelling location, however even if the declaration is inside a macro, - // the name's spelling can come from a macro argument (case 2 above). In - // this case mapping the location to the spelling location finds the - // argument's position (at `f` in MAKE_METHOD(`f`) above), which is not - // where the declaration and its comment are located. - // - // To avoid this issue, we make use of Decl::getBeginLocation() instead. - // While the declaration's position is where the name is written, the - // comment is always attached to the begining of the declaration, not to - // the name. - // - // In the first case, the begin location of the decl is outside the macro, - // at the location of `typedef`. This is where the comment is found as - // well. The begin location is not inside a macro, so it's spelling - // location is the same. - // - // In the second case, the begin location of the decl is the call to the - // macro, at `MAKE_METHOD`. However its spelling location is inside the - // the macro at the location of `void`. This is where the comment is found - // again. - // - // In the third case, there's no correct single behaviour. We want to use - // the comment outside the macro for the definition that's inside the macro. - // There is also a definition outside the macro, and we want the comment to - // apply to both. The cases we care about here is NS_ENUM() and - // NS_OPTIONS(). In general, if an enum is defined inside a macro, we should - // try to find the comment there. - - // This is handling case 3 for NS_ENUM() and NS_OPTIONS(), which define - // enum types inside the macro. - if (isa(D)) { - SourceLocation MacroCallLoc = SourceMgr.getExpansionLoc(DeclLoc); - if (auto BufferRef = - SourceMgr.getBufferOrNone(SourceMgr.getFileID(MacroCallLoc)); - BufferRef.has_value()) { - llvm::StringRef buffer = BufferRef->getBuffer().substr( - SourceMgr.getFileOffset(MacroCallLoc)); - if (buffer.starts_with("NS_ENUM(") || - buffer.starts_with("NS_OPTIONS(")) { - // We want to use the comment on the call to NS_ENUM and NS_OPTIONS - // macros for the types defined inside the macros, which is at the - // expansion location. - return MacroCallLoc; - } - } + if (!D->getLocation().isMacroID()) { + Locations.emplace_back(BaseLocation); + } else { + const auto *DeclCtx = D->getDeclContext(); + + // When encountering definitions generated from a macro (that are not + // contained by another declaration in the macro) we need to try and find + // the comment at the location of the expansion but if there is no comment + // there we should retry to see if there is a comment inside the macro as + // well. To this end we return first BaseLocation to first look at the + // expansion site, the second value is the spelling location of the + // beginning of the declaration defined inside the macro. + if (!(DeclCtx && + Decl::castFromDeclContext(DeclCtx)->getLocation().isMacroID())) { + Locations.emplace_back(SourceMgr.getExpansionLoc(BaseLocation)); } - return SourceMgr.getSpellingLoc(D->getBeginLoc()); + + // We use Decl::getBeginLoc() and not just BaseLocation here to ensure that + // we don't refer to the macro argument location at the expansion site (this + // can happen if the name's spelling is provided via macro argument), and + // always to the declaration itself. + Locations.emplace_back(SourceMgr.getSpellingLoc(D->getBeginLoc())); } - return DeclLoc; + return Locations; } RawComment *ASTContext::getRawCommentForDeclNoCacheImpl( @@ -357,30 +290,36 @@ RawComment *ASTContext::getRawCommentForDeclNoCacheImpl( } RawComment *ASTContext::getRawCommentForDeclNoCache(const Decl *D) const { - const SourceLocation DeclLoc = getDeclLocForCommentSearch(D, SourceMgr); + const auto DeclLocs = getDeclLocsForCommentSearch(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; + for (const auto DeclLoc : DeclLocs) { + // If the declaration doesn't map directly to a location in a file, we + // can't find the comment. + if (DeclLoc.isInvalid() || !DeclLoc.isFileID()) + continue; - if (ExternalSource && !CommentsLoaded) { - ExternalSource->ReadComments(); - CommentsLoaded = true; - } + if (ExternalSource && !CommentsLoaded) { + ExternalSource->ReadComments(); + CommentsLoaded = true; + } - if (Comments.empty()) - return nullptr; + if (Comments.empty()) + continue; - const FileID File = SourceMgr.getDecomposedLoc(DeclLoc).first; - if (!File.isValid()) { - return nullptr; + const FileID File = SourceMgr.getDecomposedLoc(DeclLoc).first; + if (!File.isValid()) + continue; + + const auto CommentsInThisFile = Comments.getCommentsInFile(File); + if (!CommentsInThisFile || CommentsInThisFile->empty()) + continue; + + if (RawComment *Comment = + getRawCommentForDeclNoCacheImpl(D, DeclLoc, *CommentsInThisFile)) + return Comment; } - const auto CommentsInThisFile = Comments.getCommentsInFile(File); - if (!CommentsInThisFile || CommentsInThisFile->empty()) - return nullptr; - return getRawCommentForDeclNoCacheImpl(D, DeclLoc, *CommentsInThisFile); + return nullptr; } void ASTContext::addComment(const RawComment &RC) { @@ -584,7 +523,6 @@ void ASTContext::attachCommentsToJustParsedDecls(ArrayRef Decls, // 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()) @@ -592,19 +530,22 @@ void ASTContext::attachCommentsToJustParsedDecls(ArrayRef Decls, 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; + const auto DeclLocs = getDeclLocsForCommentSearch(D, SourceMgr); + + for (const auto DeclLoc : DeclLocs) { + if (DeclLoc.isInvalid() || !DeclLoc.isFileID()) + continue; + + if (RawComment *const DocComment = getRawCommentForDeclNoCacheImpl( + D, DeclLoc, *CommentsInThisFile)) { + cacheRawCommentForDecl(*D, *DocComment); + comments::FullComment *FC = DocComment->parse(*this, PP, D); + ParsedComments[D->getCanonicalDecl()] = FC; + break; + } } } } diff --git a/clang/test/Index/annotate-comments-objc.m b/clang/test/Index/annotate-comments-objc.m index 6a48d9ae8f2cb..f013684c1a638 100644 --- a/clang/test/Index/annotate-comments-objc.m +++ b/clang/test/Index/annotate-comments-objc.m @@ -46,18 +46,23 @@ - (void)method1_isdoxy4; /*!< method1_isdoxy4 IS_DOXYGEN_SINGLE */ // attach unrelated comments in the following cases where tag decls are // embedded in declarators. -#define DECLARE_FUNCTIONS(suffix) \ - /** functionFromMacro IS_DOXYGEN_SINGLE */ \ - void functionFromMacro(void) { \ - typedef struct Struct_notdoxy Struct_notdoxy; \ - } \ - /** functionFromMacroWithSuffix IS_DOXYGEN_SINGLE */ \ - void functionFromMacro##suffix(void) { \ - typedef struct Struct_notdoxy Struct_notdoxy; \ - } - -/// IS_DOXYGEN_NOT_ATTACHED -DECLARE_FUNCTIONS(WithSuffix) +#define DECLARE_FUNCTIONS_COMMENTS_IN_MACRO(suffix) \ + /** functionFromMacro IS_DOXYGEN_SINGLE */ \ + void functionFromMacro(void) { \ + typedef struct Struct_notdoxy Struct_notdoxy; \ + } \ + /** functionFromMacroWithSuffix IS_DOXYGEN_SINGLE */ \ + void functionFromMacro##suffix(void) { \ + typedef struct Struct_notdoxy Struct_notdoxy; \ + } + +DECLARE_FUNCTIONS_COMMENTS_IN_MACRO(WithSuffix) + +#define DECLARE_FUNCTIONS \ + void functionFromMacroWithCommentFromExpansionSite(void) { typedef struct Struct_notdoxy Struct_notdoxy; } + +/// functionFromMacroWithCommentFromExpansionSite IS_DOXYGEN_SINGLE +DECLARE_FUNCTIONS /// typedef_isdoxy1 IS_DOXYGEN_SINGLE typedef struct Struct_notdoxy *typedef_isdoxy1; @@ -68,9 +73,14 @@ void functionFromMacro(void) { \ /** namedEnumFromMacro IS_DOXYGEN_SINGLE */ \ enum name { B }; -/// IS_DOXYGEN_NOT_ATTACHED DECLARE_ENUMS(namedEnumFromMacro) +#define MYENUM(name) enum name +struct Foo { + /// Vehicles IS_DOXYGEN_SINGLE + MYENUM(Vehicles) { Car, Motorbike, Boat} a; +}; + #endif // RUN: rm -rf %t @@ -133,8 +143,10 @@ void functionFromMacro(void) { \ // CHECK: annotate-comments-objc.m:41:22: EnumDecl=An_NS_ENUM_isdoxy1:{{.*}} An_NS_ENUM_isdoxy1 IS_DOXYGEN_SINGLE // CHECK: annotate-comments-objc.m:41:22: TypedefDecl=An_NS_ENUM_isdoxy1:{{.*}} An_NS_ENUM_isdoxy1 IS_DOXYGEN_SINGLE // CHECK: annotate-comments-objc.m:41:22: EnumDecl=An_NS_ENUM_isdoxy1:{{.*}} An_NS_ENUM_isdoxy1 IS_DOXYGEN_SINGLE -// CHECK: annotate-comments-objc.m:60:1: FunctionDecl=functionFromMacro:{{.*}} BriefComment=[functionFromMacro IS_DOXYGEN_SINGLE] -// CHECK: annotate-comments-objc.m:60:1: FunctionDecl=functionFromMacroWithSuffix:{{.*}} BriefComment=[functionFromMacroWithSuffix IS_DOXYGEN_SINGLE] -// CHECK: annotate-comments-objc.m:63:32: TypedefDecl=typedef_isdoxy1:{{.*}} typedef_isdoxy1 IS_DOXYGEN_SINGLE -// CHECK: annotate-comments-objc.m:72:1: EnumDecl=enumFromMacro:{{.*}} BriefComment=[enumFromMacro IS_DOXYGEN_SINGLE] -// CHECK: annotate-comments-objc.m:72:15: EnumDecl=namedEnumFromMacro:{{.*}} BriefComment=[namedEnumFromMacro IS_DOXYGEN_SINGLE] +// CHECK: annotate-comments-objc.m:59:1: FunctionDecl=functionFromMacro:{{.*}} BriefComment=[functionFromMacro IS_DOXYGEN_SINGLE] +// CHECK: annotate-comments-objc.m:59:1: FunctionDecl=functionFromMacroWithSuffix:{{.*}} BriefComment=[functionFromMacroWithSuffix IS_DOXYGEN_SINGLE] +// CHECK: annotate-comments-objc.m:65:1: FunctionDecl=functionFromMacroWithCommentFromExpansionSite:{{.*}} BriefComment=[functionFromMacroWithCommentFromExpansionSite IS_DOXYGEN_SINGLE] +// CHECK: annotate-comments-objc.m:68:32: TypedefDecl=typedef_isdoxy1:{{.*}} typedef_isdoxy1 IS_DOXYGEN_SINGLE +// CHECK: annotate-comments-objc.m:76:1: EnumDecl=enumFromMacro:{{.*}} BriefComment=[enumFromMacro IS_DOXYGEN_SINGLE] +// CHECK: annotate-comments-objc.m:76:15: EnumDecl=namedEnumFromMacro:{{.*}} BriefComment=[namedEnumFromMacro IS_DOXYGEN_SINGLE] +// CHECK: annotate-comments-objc.m:81:10: EnumDecl=Vehicles:{{.*}} Vehicles IS_DOXYGEN_SINGLE diff --git a/clang/unittests/Tooling/SourceCodeTest.cpp b/clang/unittests/Tooling/SourceCodeTest.cpp index 3d1dbceb63a7f..3641d2ee453f4 100644 --- a/clang/unittests/Tooling/SourceCodeTest.cpp +++ b/clang/unittests/Tooling/SourceCodeTest.cpp @@ -372,13 +372,11 @@ TEST(SourceCodeTest, getAssociatedRangeWithComments) { #define DECL /* Comment */ int x $r[[DECL;]])cpp"); - // Does not include comments when only the decl or the comment come from a - // macro. - // FIXME: Change code to allow this. Visit(R"cpp( #define DECL int x - // Comment - $r[[DECL;]])cpp"); + $r[[// Comment + DECL;]])cpp"); + // Does not include comments when only the comment come from a macro. Visit(R"cpp( #define COMMENT /* Comment */ COMMENT