danakj updated this revision to Diff 492176.
danakj added a comment.
Improve the comment
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D142560/new/
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
@@ -48,13 +48,18 @@
// attach unrelated comments in the following cases where tag decls are
// 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;
@@ -121,5 +126,6 @@
// 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,96 @@
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) 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<EnumDecl>(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;
+ }
+ }
}
+ return SourceMgr.getSpellingLoc(D->getBeginLoc());
}
return DeclLoc;
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
