danakj created this revision. Herald added a subscriber: arphaman. Herald added a project: All. danakj requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits.
The key part of getRawCommentForDecl() required to find a comment is determining where to look for it. The location of the decl itself is usually right, except when macros get involved. The comment in the macro is stored in RawCommentList at the spelling location of the decl, not at the place where the decl comes into being as the macro is instantiated. getDeclLocForCommentSearch() already contained to branches to try handle comments inside macros, and we are able to replace them and handle more cases as well, by returning the spelling location of the decl's begin location. That is: SourceMgr.getSpellingLoc(D->getBeginLoc()) Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D142560 Files: clang/lib/AST/ASTContext.cpp clang/test/Index/annotate-comments-objc.m
Index: clang/test/Index/annotate-comments-objc.m =================================================================== --- clang/test/Index/annotate-comments-objc.m +++ clang/test/Index/annotate-comments-objc.m @@ -46,15 +46,20 @@ // NS_ENUM macro, it is tempting to use the fact that enum decl is embedded in // the typedef. Make sure that the heuristic is strong enough that it does not // attach unrelated comments in the following cases where tag decls are -// embedded in declarators. +// embedded in declarators -#define DECLARE_FUNCTION() \ +#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_FUNCTION() +DECLARE_FUNCTIONS(WithSuffix) /// typedef_isdoxy1 IS_DOXYGEN_SINGLE typedef struct Struct_notdoxy *typedef_isdoxy1; @@ -118,8 +123,7 @@ // CHECK: annotate-comments-objc.m:30:9: ObjCInstanceMethodDecl=method1_isdoxy2:{{.*}} method1_isdoxy2 IS_DOXYGEN_SINGLE // CHECK: annotate-comments-objc.m:31:9: ObjCInstanceMethodDecl=method1_isdoxy3:{{.*}} method1_isdoxy3 IS_DOXYGEN_SINGLE // CHECK: annotate-comments-objc.m:32:9: ObjCInstanceMethodDecl=method1_isdoxy4:{{.*}} method1_isdoxy4 IS_DOXYGEN_SINGLE -// CHECK: annotate-comments-objc.m:43:22: EnumDecl=An_NS_ENUM_isdoxy1:{{.*}} An_NS_ENUM_isdoxy1 IS_DOXYGEN_SINGLE // CHECK: annotate-comments-objc.m:43:22: TypedefDecl=An_NS_ENUM_isdoxy1:{{.*}} An_NS_ENUM_isdoxy1 IS_DOXYGEN_SINGLE -// CHECK: annotate-comments-objc.m:43:22: EnumDecl=An_NS_ENUM_isdoxy1:{{.*}} An_NS_ENUM_isdoxy1 IS_DOXYGEN_SINGLE -// CHECK: annotate-comments-objc.m:60:32: TypedefDecl=typedef_isdoxy1:{{.*}} typedef_isdoxy1 IS_DOXYGEN_SINGLE - +// CHECK: annotate-comments-objc.m:62:1: FunctionDecl=functionFromMacro:{{.*}} BriefComment=[functionFromMacro IS_DOXYGEN_SINGLE] +// CHECK: annotate-comments-objc.m:62:1: FunctionDecl=functionFromMacroWithSuffix:{{.*}} BriefComment=[functionFromMacroWithSuffix IS_DOXYGEN_SINGLE] +// CHECK: annotate-comments-objc.m:65:32: TypedefDecl=typedef_isdoxy1:{{.*}} typedef_isdoxy1 IS_DOXYGEN_SINGLE Index: clang/lib/AST/ASTContext.cpp =================================================================== --- clang/lib/AST/ASTContext.cpp +++ clang/lib/AST/ASTContext.cpp @@ -181,22 +181,56 @@ 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". - return D->getBeginLoc(); - } - - 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()) - return SourceMgr.getExpansionLoc(DeclLoc); - } + // There are (at least) two types of macros we care about here. + // + // 1. Macros that are used to define a type, with a comment attached at + // the macro call site. + // ``` + // // Comment is here, where we use the macro. + // typedef NS_ENUM(NSInteger, Size) { + // SizeWidth, + // SizeHeight + // }; + // ``` + // 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) + // } + // ``` + // + // 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. + return SourceMgr.getSpellingLoc(D->getBeginLoc()); } return DeclLoc;
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits