[clang] [clang] Prioritze decl comments from macro expansion site (PR #65481)
Daniel Grumberg via cfe-commits
cfe-commits at lists.llvm.org
Wed Sep 6 06:57:35 PDT 2023
https://github.com/daniel-grumberg created https://github.com/llvm/llvm-project/pull/65481:
For declarations declared inside a macro, e.g.:
```
`#define MAKE_FUNC(suffix) \
/// Not selected doc comment \
void func_##suffix(void) { }
/// Doc comment foo
MAKE_FUNC(foo)
/// Doc comment bar
MAKE_FUNC(bar)
````
Prefer the doc comment at the expansion site instead of the one defined in the macro.
rdar://113995729
>From 7bfaa68b33e254d8a77b6dab2db7e89ef24b1175 Mon Sep 17 00:00:00 2001
From: Daniel Grumberg <dgrumberg at apple.com>
Date: Wed, 6 Sep 2023 12:20:30 +0100
Subject: [PATCH] [clang] Prioritze decl comments from macro expansion site
For declarations declared inside a macro, e.g.:
```
`#define MAKE_FUNC(suffix) \
/// Not selected doc comment \
void func_##suffix(void) { }
/// Doc comment foo
MAKE_FUNC(foo)
/// Doc comment bar
MAKE_FUNC(bar)
````
Prefer the doc comment at the expansion site instead of the one defined
in the macro.
rdar://113995729
---
clang/lib/AST/ASTContext.cpp | 191 ++++++++--------------
clang/test/Index/annotate-comments-objc.m | 5 +-
2 files changed, 70 insertions(+), 126 deletions(-)
diff --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp
index 4b1d9e86797b778..304312d3a8acfa5 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<SourceLocation, 2>
+getDeclLocsForCommentSearch(const Decl *D, SourceManager &SourceMgr) {
assert(D);
// User can not attach documentation to implicit declarations.
@@ -179,103 +179,38 @@ static SourceLocation getDeclLocForCommentSearch(const Decl *D,
isa<ClassTemplateSpecializationDecl>(D) ||
// Allow association with Y across {} in `typedef struct X {} Y`.
isa<TypedefDecl>(D))
- return D->getBeginLoc();
+ return {D->getBeginLoc()};
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<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;
- }
- }
+ // When encountering definitions generated from a macro we need to try and
+ // findthe 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 Decl::getLocation() 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 (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(), SourceMgr.getSpellingLoc(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),
+ SourceMgr.getSpellingLoc(D->getBeginLoc())};
}
- return SourceMgr.getSpellingLoc(D->getBeginLoc());
+
+ return {DeclLoc, SourceMgr.getSpellingLoc(D->getBeginLoc())};
}
- return DeclLoc;
+ return {DeclLoc};
}
RawComment *ASTContext::getRawCommentForDeclNoCacheImpl(
@@ -357,30 +292,37 @@ RawComment *ASTContext::getRawCommentForDeclNoCacheImpl(
}
RawComment *ASTContext::getRawCommentForDeclNoCache(const Decl *D) const {
- const SourceLocation DeclLoc = getDeclLocForCommentSearch(D, SourceMgr);
+ const auto DeclLocs = getDeclLocsForCommentSearch(D, SourceMgr);
+ RawComment *Ret = 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 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())
+ Ret = nullptr;
- if (ExternalSource && !CommentsLoaded) {
- ExternalSource->ReadComments();
- CommentsLoaded = true;
- }
+ if (ExternalSource && !CommentsLoaded) {
+ ExternalSource->ReadComments();
+ CommentsLoaded = true;
+ }
- if (Comments.empty())
- return nullptr;
+ if (Comments.empty())
+ Ret = nullptr;
- const FileID File = SourceMgr.getDecomposedLoc(DeclLoc).first;
- if (!File.isValid()) {
- return nullptr;
+ const FileID File = SourceMgr.getDecomposedLoc(DeclLoc).first;
+ if (!File.isValid())
+ Ret = nullptr;
+
+ const auto CommentsInThisFile = Comments.getCommentsInFile(File);
+ if (!CommentsInThisFile || CommentsInThisFile->empty())
+ Ret = nullptr;
+
+ if ((Ret =
+ getRawCommentForDeclNoCacheImpl(D, DeclLoc, *CommentsInThisFile)))
+ return Ret;
}
- const auto CommentsInThisFile = Comments.getCommentsInFile(File);
- if (!CommentsInThisFile || CommentsInThisFile->empty())
- return nullptr;
- return getRawCommentForDeclNoCacheImpl(D, DeclLoc, *CommentsInThisFile);
+ return Ret;
}
void ASTContext::addComment(const RawComment &RC) {
@@ -592,19 +534,22 @@ void ASTContext::attachCommentsToJustParsedDecls(ArrayRef<Decl *> Decls,
D = &adjustDeclToTemplate(*D);
- const SourceLocation DeclLoc = getDeclLocForCommentSearch(D, SourceMgr);
+ const auto DeclLocs = getDeclLocsForCommentSearch(D, SourceMgr);
- if (DeclLoc.isInvalid() || !DeclLoc.isFileID())
- continue;
+ for (const auto DeclLoc : DeclLocs) {
+ if (DeclLoc.isInvalid() || !DeclLoc.isFileID())
+ continue;
- if (DeclRawComments.count(D) > 0)
- 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;
+ 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 6a48d9ae8f2cb3e..bd13ac3fba8b8ac 100644
--- a/clang/test/Index/annotate-comments-objc.m
+++ b/clang/test/Index/annotate-comments-objc.m
@@ -68,7 +68,6 @@ void functionFromMacro(void) { \
/** namedEnumFromMacro IS_DOXYGEN_SINGLE */ \
enum name { B };
-/// IS_DOXYGEN_NOT_ATTACHED
DECLARE_ENUMS(namedEnumFromMacro)
#endif
@@ -136,5 +135,5 @@ void functionFromMacro(void) { \
// 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:71:1: EnumDecl=enumFromMacro:{{.*}} BriefComment=[enumFromMacro IS_DOXYGEN_SINGLE]
+// CHECK: annotate-comments-objc.m:71:15: EnumDecl=namedEnumFromMacro:{{.*}} BriefComment=[namedEnumFromMacro IS_DOXYGEN_SINGLE]
More information about the cfe-commits
mailing list