[clang] [Clang][Comments] Attach comments to decl even if preproc directives are in between (PR #88367)
https://github.com/hdoc updated https://github.com/llvm/llvm-project/pull/88367
>From a98d117c68b33d2e6632a832ff77b0e8258469e6 Mon Sep 17 00:00:00 2001
From: hdoc
Date: Thu, 11 Apr 2024 01:54:18 -0700
Subject: [PATCH 1/3] Attach comments to decl even if preproc directives are in
between
---
clang/lib/AST/ASTContext.cpp | 7 +-
clang/lib/Headers/amxcomplexintrin.h | 4 +
clang/lib/Headers/ia32intrin.h | 2 +
clang/test/Index/annotate-comments.cpp | 5 +-
clang/unittests/AST/DeclTest.cpp | 310 +
5 files changed, 323 insertions(+), 5 deletions(-)
diff --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp
index bf74e56a14799..3e9131ecf12d0 100644
--- a/clang/lib/AST/ASTContext.cpp
+++ b/clang/lib/AST/ASTContext.cpp
@@ -282,9 +282,10 @@ RawComment *ASTContext::getRawCommentForDeclNoCacheImpl(
StringRef Text(Buffer + CommentEndOffset,
DeclLocDecomp.second - CommentEndOffset);
- // There should be no other declarations or preprocessor directives between
- // comment and declaration.
- if (Text.find_last_of(";{}#@") != StringRef::npos)
+ // There should be no other declarations between comment and declaration.
+ // Preprocessor directives are implicitly allowed to be between a comment and
+ // its associated decl.
+ if (Text.find_last_of(";{}@") != StringRef::npos)
return nullptr;
return CommentBeforeDecl;
diff --git a/clang/lib/Headers/amxcomplexintrin.h
b/clang/lib/Headers/amxcomplexintrin.h
index 84ef972fcadf0..19eaee10ede65 100644
--- a/clang/lib/Headers/amxcomplexintrin.h
+++ b/clang/lib/Headers/amxcomplexintrin.h
@@ -62,6 +62,8 @@
///The 2nd source tile. Max size is 1024 Bytes.
#define _tile_cmmimfp16ps(dst, a, b) __builtin_ia32_tcmmimfp16ps(dst, a, b)
+;
+
/// Perform matrix multiplication of two tiles containing complex elements and
///accumulate the results into a packed single precision tile. Each dword
///element in input tiles \a a and \a b is interpreted as a complex number
@@ -107,6 +109,8 @@
///The 2nd source tile. Max size is 1024 Bytes.
#define _tile_cmmrlfp16ps(dst, a, b) __builtin_ia32_tcmmrlfp16ps(dst, a, b)
+;
+
static __inline__ _tile1024i __DEFAULT_FN_ATTRS_COMPLEX
_tile_cmmimfp16ps_internal(unsigned short m, unsigned short n, unsigned short
k,
_tile1024i dst, _tile1024i src1, _tile1024i src2) {
diff --git a/clang/lib/Headers/ia32intrin.h b/clang/lib/Headers/ia32intrin.h
index 8e65f232a0def..c9c92b9ee0f99 100644
--- a/clang/lib/Headers/ia32intrin.h
+++ b/clang/lib/Headers/ia32intrin.h
@@ -533,6 +533,8 @@ __rdtscp(unsigned int *__A) {
/// \see __rdpmc
#define _rdpmc(A) __rdpmc(A)
+;
+
static __inline__ void __DEFAULT_FN_ATTRS
_wbinvd(void) {
__builtin_ia32_wbinvd();
diff --git a/clang/test/Index/annotate-comments.cpp
b/clang/test/Index/annotate-comments.cpp
index 6f9f8f0bbbc9e..bff25d46cf80e 100644
--- a/clang/test/Index/annotate-comments.cpp
+++ b/clang/test/Index/annotate-comments.cpp
@@ -204,9 +204,9 @@ void isdoxy45(void);
/// Ggg. IS_DOXYGEN_END
void isdoxy46(void);
-/// IS_DOXYGEN_NOT_ATTACHED
+/// isdoxy47 IS_DOXYGEN_SINGLE
#define FOO
-void notdoxy47(void);
+void isdoxy47(void);
/// IS_DOXYGEN_START Aaa bbb
/// \param ccc
@@ -330,6 +330,7 @@ void isdoxy54(int);
// CHECK: annotate-comments.cpp:185:6: FunctionDecl=isdoxy44:{{.*}}
BriefComment=[IS_DOXYGEN_START Aaa bbb ccc.]
// CHECK: annotate-comments.cpp:195:6: FunctionDecl=isdoxy45:{{.*}}
BriefComment=[Ddd eee. Fff.]
// CHECK: annotate-comments.cpp:205:6: FunctionDecl=isdoxy46:{{.*}}
BriefComment=[Ddd eee. Fff.]
+// CHECK: annotate-comments.cpp:209:6: FunctionDecl=isdoxy47:{{.*}} isdoxy47
IS_DOXYGEN_SINGLE
// CHECK: annotate-comments.cpp:214:6: FunctionDecl=isdoxy48:{{.*}}
BriefComment=[IS_DOXYGEN_START Aaa bbb]
// CHECK: annotate-comments.cpp:218:6: FunctionDecl=isdoxy49:{{.*}}
BriefComment=[IS_DOXYGEN_START Aaa]
// CHECK: annotate-comments.cpp:222:6: FunctionDecl=isdoxy50:{{.*}}
BriefComment=[Returns ddd IS_DOXYGEN_END]
diff --git a/clang/unittests/AST/DeclTest.cpp b/clang/unittests/AST/DeclTest.cpp
index 16aa2b50b7a06..8dca011ba8779 100644
--- a/clang/unittests/AST/DeclTest.cpp
+++ b/clang/unittests/AST/DeclTest.cpp
@@ -576,3 +576,313 @@ void instantiate_template() {
EXPECT_EQ(GetNameInfoRange(Matches[1]), "");
EXPECT_EQ(GetNameInfoRange(Matches[2]), "");
}
+
+TEST(Decl, CommentsAttachedToDecl1) {
+ const SmallVector Sources{
+ R"(
+/// Test comment
+void f();
+ )",
+
+ R"(
+/// Test comment
+
+void f();
+ )",
+
+ R"(
+/// Test comment
+#if 0
+// tralala
+#endif
+void f();
+ )",
+
+ R"(
+/// Test comment
+
+#if 0
+// tralala
+#endif
+
+void f();
+ )",
+
+ R"(
+/// Test comment
+#ifdef DOCS
+template
+#endif
+void f();
+
[clang] [Clang][Comments] Attach comments to decl even if preproc directives are in between (PR #88367)
https://github.com/AaronBallman edited https://github.com/llvm/llvm-project/pull/88367 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][Comments] Attach comments to decl even if preproc directives are in between (PR #88367)
https://github.com/AaronBallman commented: Changes generally LG aside from an update to a test, but this should also have a release note in `clang/docs/ReleaseNotes.rst` so users know about the new functionality and the limitations regarding macros. https://github.com/llvm/llvm-project/pull/88367 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][Comments] Attach comments to decl even if preproc directives are in between (PR #88367)
@@ -533,6 +533,8 @@ __rdtscp(unsigned int *__A) {
/// \see __rdpmc
#define _rdpmc(A) __rdpmc(A)
+;
AaronBallman wrote:
As far as the changes go, that is fine. I do still think we need to change the
underlying comment parsing at some point (not as part of this PR) though
because I don't think this will be very obvious to users.
https://github.com/llvm/llvm-project/pull/88367
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][Comments] Attach comments to decl even if preproc directives are in between (PR #88367)
@@ -107,6 +107,24 @@ ///The 2nd source tile. Max size is 1024 Bytes. #define _tile_cmmrlfp16ps(dst, a, b) __builtin_ia32_tcmmrlfp16ps(dst, a, b) +/// Perform matrix multiplication of two tiles containing complex elements and AaronBallman wrote: Thank you for the explanation! https://github.com/llvm/llvm-project/pull/88367 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][Comments] Attach comments to decl even if preproc directives are in between (PR #88367)
@@ -533,6 +533,8 @@ __rdtscp(unsigned int *__A) {
/// \see __rdpmc
#define _rdpmc(A) __rdpmc(A)
+;
hdoc wrote:
If it sounds reasonable to you, I can fix this with the same method used in the
`clang/lib/Headers/amxcomplexintrin.h` file. Unfortunately changing the
underlying comment parsing functionality would be a complicated change.
https://github.com/llvm/llvm-project/pull/88367
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][Comments] Attach comments to decl even if preproc directives are in between (PR #88367)
@@ -107,6 +107,24 @@ ///The 2nd source tile. Max size is 1024 Bytes. #define _tile_cmmrlfp16ps(dst, a, b) __builtin_ia32_tcmmrlfp16ps(dst, a, b) +/// Perform matrix multiplication of two tiles containing complex elements and hdoc wrote: This is not an unrelated change, I describe the issue in the "Complications" section of the PR body. In short, the `-Wdocumentation` warning gets triggered here because the macro for the `#define` on line 108 has a doc comment which Clang attributes to the function on line 128. Since the params documented in the macro's doc comment don't match the params in the function, `-Wdocumentation` throws an error. As a hack, I originally put semicolons on their own line because that effectively splits the decls from `-Wdocumentation`'s point of view, but it's a bit of a dirty hack. Instead, I just added a proper doc comment for the function here which stops the warning without changing the underlying comment parsing functionality. Still a workaround, but cleaner. Changing that underlying functionality would be a much more involved since the comment parser operates on the AST, at which point most of the preproc info is gone because of how Clang is structured (please correct me if I'm wrong here). https://github.com/llvm/llvm-project/pull/88367 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][Comments] Attach comments to decl even if preproc directives are in between (PR #88367)
@@ -107,6 +107,24 @@ ///The 2nd source tile. Max size is 1024 Bytes. #define _tile_cmmrlfp16ps(dst, a, b) __builtin_ia32_tcmmrlfp16ps(dst, a, b) +/// Perform matrix multiplication of two tiles containing complex elements and AaronBallman wrote: Is this an unrelated change? https://github.com/llvm/llvm-project/pull/88367 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][Comments] Attach comments to decl even if preproc directives are in between (PR #88367)
@@ -533,6 +533,8 @@ __rdtscp(unsigned int *__A) {
/// \see __rdpmc
#define _rdpmc(A) __rdpmc(A)
+;
AaronBallman wrote:
This is quite surprising; I'm guessing this is so that the previous comment
does not associate with the function declared below, but this still feels
pretty unclean.
https://github.com/llvm/llvm-project/pull/88367
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][Comments] Attach comments to decl even if preproc directives are in between (PR #88367)
https://github.com/AaronBallman edited https://github.com/llvm/llvm-project/pull/88367 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][Comments] Attach comments to decl even if preproc directives are in between (PR #88367)
https://github.com/AaronBallman commented: Because doxygen supports documenting macros (https://www.doxygen.nl/manual/commands.html#cmddef), I am worried how often this will cause us to associate comments incorrectly on the declaration. I wonder if we should be a bit smarter and check for `#define` at the start of a line when we encounter a `#`. e.g., ``` /*! \def DERP(x) Does a derpy thing with x */ #define DERP(x) (x) void derp(void); // Does not get the doxygen comment ``` while ``` /// Does amazing things, like works in the presence of /// #define doing stupid things. void func(); // does get the doxygen comment ``` https://github.com/llvm/llvm-project/pull/88367 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][Comments] Attach comments to decl even if preproc directives are in between (PR #88367)
llvmbot wrote:
@llvm/pr-subscribers-backend-x86
@llvm/pr-subscribers-clang
Author: hdoc (hdoc)
Changes
### Background
It's surprisingly common for C++ code in the wild to conditionally show/hide
declarations to Doxygen through the use of preprocessor directives. One
especially common version of this pattern is demonstrated below:
```cpp
/// @brief Test comment
#ifdef DOXYGEN_BUILD_ENABLED
templatetypename T
#else
template typename T
typename std::enable_ifstd::is_integralT::value::type
#endif
void f() {}
```
There are more examples I've collected below to demonstrate usage of this
pattern:
- Example 1:
[Magnum](https://github.com/mosra/magnum/blob/8538610fa27e1db37070eaabe34f1e4e41648bab/src/Magnum/Resource.h#L117-L127)
- Example 2:
[libcds](https://github.com/khizmax/libcds/blob/9985d2a87feaa3e92532e28f4ab762a82855a49c/cds/container/michael_list_nogc.h#L36-L54)
- Example 3:
[rocPRIM](https://github.com/ROCm/rocPRIM/blob/609ae19565ff6a3499168b76a0be5652762e24f6/rocprim/include/rocprim/block/detail/block_reduce_raking_reduce.hpp#L60-L65)
>From my research, it seems like the most common rationale for this
>functionality is hiding difficult-to-parse code from Doxygen, especially where
>template metaprogramming is concerned.
Currently, Clang does not support attaching comments to decls if there are
preprocessor comments between the comment and the decl. This is enforced here:
https://github.com/llvm/llvm-project/blob/b6ebea7972cd05a8e4dcf0d5a54f2c79395a/clang/lib/AST/ASTContext.cpp#L284-L287
Alongside preprocessor directives, any instance of `;{}#@` between a comment
and decl will cause the comment to not be attached to the decl.
Rationale
It would be nice for Clang-based documentation tools, such as
[hdoc](https://hdoc.io), to support code using this pattern. Users expect to
see comments attached to the relevant decl — even if there is an `#ifdef` in
the way — which Clang does not currently do.
History
Originally, commas were also in the list of "banned" characters, but were
removed in `b534d3a0ef69`
([link](https://github.com/llvm/llvm-project/commit/b534d3a0ef6970f5e42f10ba5cfcb562d8b184e1))
because availability macros often have commas in them. From my reading of the
code, it appears that the original intent of the code was to exclude macros and
decorators between comments and decls, possibly in an attempt to properly
attribute comments to macros (discussed further in "Complications", below).
There's some more discussion here: https://reviews.llvm.org/D125061.
### Change
This modifies Clang comment parsing so that comments are attached to subsequent
declarations even if there are preprocessor directives between the end of the
comment and the start of the decl. Furthermore, this change:
- Adds tests to verify that comments are attached to their associated decls
even if there are preprocessor directives in between
- Adds tests to verify that current behavior has not changed (i.e. use of the
other characters between comment and decl will result in the comment not being
attached to the decl)
- Updates existing `lit` tests which would otherwise break.
Complications
Clang [does not yet support](https://github.com/llvm/llvm-project/issues/38206)
attaching doc comments to macros. Consequently, the change proposed in this RFC
affects cases where a doc comment attached to a macro is followed immediately
by a normal declaration. In these cases, the macro's doc comments will be
attached to the subsequent decl. Previously they would be ignored because any
preprocessor directives between a comment and a decl would result in the
comment not being attached to the decl. An example of this is shown below.
```cpp
/// Doc comment for a function-like macro
/// @param n
///A macro argument
#define custom_sqrt(n) __internal_sqrt(n)
int __internal_sqrt(int n) { return __builtin_sqrt(n); }
// NB: the doc comment for the custom_sqrt macro will actually be attached to
__internal_sqrt!
```
There is a real instance of this problem in the Clang codebase, namely here:
https://github.com/llvm/llvm-project/blob/be10070f91b86a6f126d2451852242bfcb2cd366/clang/lib/Headers/amxcomplexintrin.h#L65-L114
As part of this RFC, I've added a semicolon to break up the Clang comment
parsing so that the `-Wdocumentation` errors go away, but this is a hack. The
real solution is to fix Clang comment parsing so that doc comments are properly
attached to macros, however this would be a large change that is outside of the
scope of this RFC.
---
Full diff: https://github.com/llvm/llvm-project/pull/88367.diff
5 Files Affected:
- (modified) clang/lib/AST/ASTContext.cpp (+4-3)
- (modified) clang/lib/Headers/amxcomplexintrin.h (+18)
- (modified) clang/lib/Headers/ia32intrin.h (+2)
- (modified) clang/test/Index/annotate-comments.cpp (+3-2)
- (modified) clang/unittests/AST/DeclTest.cpp (+310)
``diff
diff --git a/clang/lib/AST/ASTContext.cpp
[clang] [Clang][Comments] Attach comments to decl even if preproc directives are in between (PR #88367)
hdoc wrote: @AaronBallman this one is ready for review :) https://github.com/llvm/llvm-project/pull/88367 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][Comments] Attach comments to decl even if preproc directives are in between (PR #88367)
https://github.com/hdoc ready_for_review https://github.com/llvm/llvm-project/pull/88367 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][Comments] Attach comments to decl even if preproc directives are in between (PR #88367)
AaronBallman wrote: Is this still in Draft status or should this be marked as ready for review? https://github.com/llvm/llvm-project/pull/88367 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][Comments] Attach comments to decl even if preproc directives are in between (PR #88367)
hdoc wrote: >From my reading of the Doxygen documentation it seems like anything goes >except putting the doc comment inside the actual body of a function. That >being said, I think that change is out of the scope of this PR because it >would likely require modifications to the Clang comment-to-decl matching >functionality. It's better to do that in a separate PR https://github.com/llvm/llvm-project/pull/88367 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][Comments] Attach comments to decl even if preproc directives are in between (PR #88367)
Boddlnagg wrote:
Regarding the positioning of Doxygen comments, I just saw this and was
wondering if it's possible to also support parsing of Doxygen comments that
come after the signature, which happens to be the preferred style in our
codebase ...
```c++
static int doSomething(int foobar)
/** \brief ...
* \param foobar foo bar
* \return return value
*/
{
...
```
Doxygen accepts this, although I don't know if it's specified/documented as
such.
https://github.com/llvm/llvm-project/pull/88367
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][Comments] Attach comments to decl even if preproc directives are in between (PR #88367)
@@ -62,6 +62,8 @@ ///The 2nd source tile. Max size is 1024 Bytes. #define _tile_cmmimfp16ps(dst, a, b) __builtin_ia32_tcmmimfp16ps(dst, a, b) +; hdoc wrote: This has been fixed in the latest commit, where the internal function is documented with a Doxygen doc comment so that the `-Wdocumentation` warnings are not triggered. https://github.com/llvm/llvm-project/pull/88367 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][Comments] Attach comments to decl even if preproc directives are in between (PR #88367)
hdoc wrote: Ping https://github.com/llvm/llvm-project/pull/88367 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][Comments] Attach comments to decl even if preproc directives are in between (PR #88367)
https://github.com/hdoc updated https://github.com/llvm/llvm-project/pull/88367
>From 61612c5f340e25198deaf68e6904323955efe489 Mon Sep 17 00:00:00 2001
From: hdoc
Date: Thu, 11 Apr 2024 01:54:18 -0700
Subject: [PATCH 1/2] Attach comments to decl even if preproc directives are in
between
---
clang/lib/AST/ASTContext.cpp | 7 +-
clang/lib/Headers/amxcomplexintrin.h | 4 +
clang/lib/Headers/ia32intrin.h | 2 +
clang/test/Index/annotate-comments.cpp | 5 +-
clang/unittests/AST/DeclTest.cpp | 310 +
5 files changed, 323 insertions(+), 5 deletions(-)
diff --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp
index 2fa6aedca4c6ab..1c8d55b8627452 100644
--- a/clang/lib/AST/ASTContext.cpp
+++ b/clang/lib/AST/ASTContext.cpp
@@ -282,9 +282,10 @@ RawComment *ASTContext::getRawCommentForDeclNoCacheImpl(
StringRef Text(Buffer + CommentEndOffset,
DeclLocDecomp.second - CommentEndOffset);
- // There should be no other declarations or preprocessor directives between
- // comment and declaration.
- if (Text.find_last_of(";{}#@") != StringRef::npos)
+ // There should be no other declarations between comment and declaration.
+ // Preprocessor directives are implicitly allowed to be between a comment and
+ // its associated decl.
+ if (Text.find_last_of(";{}@") != StringRef::npos)
return nullptr;
return CommentBeforeDecl;
diff --git a/clang/lib/Headers/amxcomplexintrin.h
b/clang/lib/Headers/amxcomplexintrin.h
index 84ef972fcadf03..19eaee10ede659 100644
--- a/clang/lib/Headers/amxcomplexintrin.h
+++ b/clang/lib/Headers/amxcomplexintrin.h
@@ -62,6 +62,8 @@
///The 2nd source tile. Max size is 1024 Bytes.
#define _tile_cmmimfp16ps(dst, a, b) __builtin_ia32_tcmmimfp16ps(dst, a, b)
+;
+
/// Perform matrix multiplication of two tiles containing complex elements and
///accumulate the results into a packed single precision tile. Each dword
///element in input tiles \a a and \a b is interpreted as a complex number
@@ -107,6 +109,8 @@
///The 2nd source tile. Max size is 1024 Bytes.
#define _tile_cmmrlfp16ps(dst, a, b) __builtin_ia32_tcmmrlfp16ps(dst, a, b)
+;
+
static __inline__ _tile1024i __DEFAULT_FN_ATTRS_COMPLEX
_tile_cmmimfp16ps_internal(unsigned short m, unsigned short n, unsigned short
k,
_tile1024i dst, _tile1024i src1, _tile1024i src2) {
diff --git a/clang/lib/Headers/ia32intrin.h b/clang/lib/Headers/ia32intrin.h
index 8e65f232a0def8..c9c92b9ee0f991 100644
--- a/clang/lib/Headers/ia32intrin.h
+++ b/clang/lib/Headers/ia32intrin.h
@@ -533,6 +533,8 @@ __rdtscp(unsigned int *__A) {
/// \see __rdpmc
#define _rdpmc(A) __rdpmc(A)
+;
+
static __inline__ void __DEFAULT_FN_ATTRS
_wbinvd(void) {
__builtin_ia32_wbinvd();
diff --git a/clang/test/Index/annotate-comments.cpp
b/clang/test/Index/annotate-comments.cpp
index 6f9f8f0bbbc9e5..bff25d46cf80ee 100644
--- a/clang/test/Index/annotate-comments.cpp
+++ b/clang/test/Index/annotate-comments.cpp
@@ -204,9 +204,9 @@ void isdoxy45(void);
/// Ggg. IS_DOXYGEN_END
void isdoxy46(void);
-/// IS_DOXYGEN_NOT_ATTACHED
+/// isdoxy47 IS_DOXYGEN_SINGLE
#define FOO
-void notdoxy47(void);
+void isdoxy47(void);
/// IS_DOXYGEN_START Aaa bbb
/// \param ccc
@@ -330,6 +330,7 @@ void isdoxy54(int);
// CHECK: annotate-comments.cpp:185:6: FunctionDecl=isdoxy44:{{.*}}
BriefComment=[IS_DOXYGEN_START Aaa bbb ccc.]
// CHECK: annotate-comments.cpp:195:6: FunctionDecl=isdoxy45:{{.*}}
BriefComment=[Ddd eee. Fff.]
// CHECK: annotate-comments.cpp:205:6: FunctionDecl=isdoxy46:{{.*}}
BriefComment=[Ddd eee. Fff.]
+// CHECK: annotate-comments.cpp:209:6: FunctionDecl=isdoxy47:{{.*}} isdoxy47
IS_DOXYGEN_SINGLE
// CHECK: annotate-comments.cpp:214:6: FunctionDecl=isdoxy48:{{.*}}
BriefComment=[IS_DOXYGEN_START Aaa bbb]
// CHECK: annotate-comments.cpp:218:6: FunctionDecl=isdoxy49:{{.*}}
BriefComment=[IS_DOXYGEN_START Aaa]
// CHECK: annotate-comments.cpp:222:6: FunctionDecl=isdoxy50:{{.*}}
BriefComment=[Returns ddd IS_DOXYGEN_END]
diff --git a/clang/unittests/AST/DeclTest.cpp b/clang/unittests/AST/DeclTest.cpp
index 2530ce74eb6a37..e5c1a578864292 100644
--- a/clang/unittests/AST/DeclTest.cpp
+++ b/clang/unittests/AST/DeclTest.cpp
@@ -545,3 +545,313 @@ TEST(Decl, TemplateArgumentDefaulted) {
EXPECT_TRUE(ArgList.get(2).getIsDefaulted());
EXPECT_TRUE(ArgList.get(3).getIsDefaulted());
}
+
+TEST(Decl, CommentsAttachedToDecl1) {
+ const SmallVector Sources{
+ R"(
+/// Test comment
+void f();
+ )",
+
+ R"(
+/// Test comment
+
+void f();
+ )",
+
+ R"(
+/// Test comment
+#if 0
+// tralala
+#endif
+void f();
+ )",
+
+ R"(
+/// Test comment
+
+#if 0
+// tralala
+#endif
+
+void f();
+ )",
+
+ R"(
+/// Test comment
+#ifdef DOCS
+template
+#endif
+
[clang] [Clang][Comments] Attach comments to decl even if preproc directives are in between (PR #88367)
https://github.com/hdoc updated https://github.com/llvm/llvm-project/pull/88367
>From 61612c5f340e25198deaf68e6904323955efe489 Mon Sep 17 00:00:00 2001
From: hdoc
Date: Thu, 11 Apr 2024 01:54:18 -0700
Subject: [PATCH] Attach comments to decl even if preproc directives are in
between
---
clang/lib/AST/ASTContext.cpp | 7 +-
clang/lib/Headers/amxcomplexintrin.h | 4 +
clang/lib/Headers/ia32intrin.h | 2 +
clang/test/Index/annotate-comments.cpp | 5 +-
clang/unittests/AST/DeclTest.cpp | 310 +
5 files changed, 323 insertions(+), 5 deletions(-)
diff --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp
index 2fa6aedca4c6ab..1c8d55b8627452 100644
--- a/clang/lib/AST/ASTContext.cpp
+++ b/clang/lib/AST/ASTContext.cpp
@@ -282,9 +282,10 @@ RawComment *ASTContext::getRawCommentForDeclNoCacheImpl(
StringRef Text(Buffer + CommentEndOffset,
DeclLocDecomp.second - CommentEndOffset);
- // There should be no other declarations or preprocessor directives between
- // comment and declaration.
- if (Text.find_last_of(";{}#@") != StringRef::npos)
+ // There should be no other declarations between comment and declaration.
+ // Preprocessor directives are implicitly allowed to be between a comment and
+ // its associated decl.
+ if (Text.find_last_of(";{}@") != StringRef::npos)
return nullptr;
return CommentBeforeDecl;
diff --git a/clang/lib/Headers/amxcomplexintrin.h
b/clang/lib/Headers/amxcomplexintrin.h
index 84ef972fcadf03..19eaee10ede659 100644
--- a/clang/lib/Headers/amxcomplexintrin.h
+++ b/clang/lib/Headers/amxcomplexintrin.h
@@ -62,6 +62,8 @@
///The 2nd source tile. Max size is 1024 Bytes.
#define _tile_cmmimfp16ps(dst, a, b) __builtin_ia32_tcmmimfp16ps(dst, a, b)
+;
+
/// Perform matrix multiplication of two tiles containing complex elements and
///accumulate the results into a packed single precision tile. Each dword
///element in input tiles \a a and \a b is interpreted as a complex number
@@ -107,6 +109,8 @@
///The 2nd source tile. Max size is 1024 Bytes.
#define _tile_cmmrlfp16ps(dst, a, b) __builtin_ia32_tcmmrlfp16ps(dst, a, b)
+;
+
static __inline__ _tile1024i __DEFAULT_FN_ATTRS_COMPLEX
_tile_cmmimfp16ps_internal(unsigned short m, unsigned short n, unsigned short
k,
_tile1024i dst, _tile1024i src1, _tile1024i src2) {
diff --git a/clang/lib/Headers/ia32intrin.h b/clang/lib/Headers/ia32intrin.h
index 8e65f232a0def8..c9c92b9ee0f991 100644
--- a/clang/lib/Headers/ia32intrin.h
+++ b/clang/lib/Headers/ia32intrin.h
@@ -533,6 +533,8 @@ __rdtscp(unsigned int *__A) {
/// \see __rdpmc
#define _rdpmc(A) __rdpmc(A)
+;
+
static __inline__ void __DEFAULT_FN_ATTRS
_wbinvd(void) {
__builtin_ia32_wbinvd();
diff --git a/clang/test/Index/annotate-comments.cpp
b/clang/test/Index/annotate-comments.cpp
index 6f9f8f0bbbc9e5..bff25d46cf80ee 100644
--- a/clang/test/Index/annotate-comments.cpp
+++ b/clang/test/Index/annotate-comments.cpp
@@ -204,9 +204,9 @@ void isdoxy45(void);
/// Ggg. IS_DOXYGEN_END
void isdoxy46(void);
-/// IS_DOXYGEN_NOT_ATTACHED
+/// isdoxy47 IS_DOXYGEN_SINGLE
#define FOO
-void notdoxy47(void);
+void isdoxy47(void);
/// IS_DOXYGEN_START Aaa bbb
/// \param ccc
@@ -330,6 +330,7 @@ void isdoxy54(int);
// CHECK: annotate-comments.cpp:185:6: FunctionDecl=isdoxy44:{{.*}}
BriefComment=[IS_DOXYGEN_START Aaa bbb ccc.]
// CHECK: annotate-comments.cpp:195:6: FunctionDecl=isdoxy45:{{.*}}
BriefComment=[Ddd eee. Fff.]
// CHECK: annotate-comments.cpp:205:6: FunctionDecl=isdoxy46:{{.*}}
BriefComment=[Ddd eee. Fff.]
+// CHECK: annotate-comments.cpp:209:6: FunctionDecl=isdoxy47:{{.*}} isdoxy47
IS_DOXYGEN_SINGLE
// CHECK: annotate-comments.cpp:214:6: FunctionDecl=isdoxy48:{{.*}}
BriefComment=[IS_DOXYGEN_START Aaa bbb]
// CHECK: annotate-comments.cpp:218:6: FunctionDecl=isdoxy49:{{.*}}
BriefComment=[IS_DOXYGEN_START Aaa]
// CHECK: annotate-comments.cpp:222:6: FunctionDecl=isdoxy50:{{.*}}
BriefComment=[Returns ddd IS_DOXYGEN_END]
diff --git a/clang/unittests/AST/DeclTest.cpp b/clang/unittests/AST/DeclTest.cpp
index 2530ce74eb6a37..e5c1a578864292 100644
--- a/clang/unittests/AST/DeclTest.cpp
+++ b/clang/unittests/AST/DeclTest.cpp
@@ -545,3 +545,313 @@ TEST(Decl, TemplateArgumentDefaulted) {
EXPECT_TRUE(ArgList.get(2).getIsDefaulted());
EXPECT_TRUE(ArgList.get(3).getIsDefaulted());
}
+
+TEST(Decl, CommentsAttachedToDecl1) {
+ const SmallVector Sources{
+ R"(
+/// Test comment
+void f();
+ )",
+
+ R"(
+/// Test comment
+
+void f();
+ )",
+
+ R"(
+/// Test comment
+#if 0
+// tralala
+#endif
+void f();
+ )",
+
+ R"(
+/// Test comment
+
+#if 0
+// tralala
+#endif
+
+void f();
+ )",
+
+ R"(
+/// Test comment
+#ifdef DOCS
+template
+#endif
+
[clang] [Clang][Comments] Attach comments to decl even if preproc directives are in between (PR #88367)
@@ -62,6 +62,8 @@ ///The 2nd source tile. Max size is 1024 Bytes. #define _tile_cmmimfp16ps(dst, a, b) __builtin_ia32_tcmmimfp16ps(dst, a, b) +; hdoc wrote: This is a workaround to a Clang issue where doc comments cannot be associated with macros in Clang. The semicolon breaks up the preceding doc comment from the subsequent function. Without this, we get `-Wdocumentation` errors during the build. https://github.com/llvm/llvm-project/pull/88367 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][Comments] Attach comments to decl even if preproc directives are in between (PR #88367)
https://github.com/hdoc edited https://github.com/llvm/llvm-project/pull/88367 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][Comments] Attach comments to decl even if preproc directives are in between (PR #88367)
github-actions[bot] wrote: Thank you for submitting a Pull Request (PR) to the LLVM Project! This PR will be automatically labeled and the relevant teams will be notified. If you wish to, you can add reviewers by using the "Reviewers" section on this page. If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using `@` followed by their GitHub username. If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers. If you have further questions, they may be answered by the [LLVM GitHub User Guide](https://llvm.org/docs/GitHub.html). You can also ask questions in a comment on this PR, on the [LLVM Discord](https://discord.com/invite/xS7Z362) or on the [forums](https://discourse.llvm.org/). https://github.com/llvm/llvm-project/pull/88367 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][Comments] Attach comments to decl even if preproc directives are in between (PR #88367)
https://github.com/hdoc created https://github.com/llvm/llvm-project/pull/88367
# Background
It's surprisingly common for C++ code in the wild to conditionally show/hide
declarations to Doxygen through the use of preprocessor directives. One
especially common version of this pattern is demonstrated below:
```cpp
/// @brief Test comment
#ifdef DOXYGEN_BUILD_ENABLED
template
#else
template
typename std::enable_if::value>::type
#endif
void f() {}
```
There are more examples I've collected below to demonstrate usage of this
pattern:
- Magnum:
https://github.com/mosra/magnum/blob/8538610fa27e1db37070eaabe34f1e4e41648bab/src/Magnum/Resource.h#L117-L127
- libcds:
https://github.com/khizmax/libcds/blob/9985d2a87feaa3e92532e28f4ab762a82855a49c/cds/container/michael_list_nogc.h#L36-L54
- rocPRIM:
https://github.com/ROCm/rocPRIM/blob/609ae19565ff6a3499168b76a0be5652762e24f6/rocprim/include/rocprim/block/detail/block_reduce_raking_reduce.hpp#L60-L65
>From my research, it seems like the most common rationale for this
>functionality is hiding difficult-to-parse code from Doxygen, especially where
>template metaprogramming is concerned.
Currently, Clang does not support attaching comments to decls if there are
preprocessor comments between the comment and the decl. This is enforced here:
https://github.com/llvm/llvm-project/blob/b6ebea7972cd05a8e4dcf0d5a54f2c79395a/clang/lib/AST/ASTContext.cpp#L284-L287
Alongside preprocessor directives, any instance of `;{}#@` between a comment
and decl will cause the comment to not be attached to the decl.
## Rationale
It would be nice for Clang-based documentation tools, such as
[hdoc](https://hdoc.io), to support code using this pattern. Users expect to
see comments attached to the relevant decl — even if there is an `#ifdef` in
the way — which Clang does not currently do.
## History
Originally, commas were also in the list of "banned" characters, but were
removed in `b534d3a0ef69`
([link](https://github.com/llvm/llvm-project/commit/b534d3a0ef6970f5e42f10ba5cfcb562d8b184e1))
because availability macros often have commas in them. From my reading of the
code, it appears that the original intent of the code was to exclude macros and
decorators between comments and decls, possibly in an attempt to properly
attribute comments to macros (discussed further in "Complications", below).
There's some more discussion here: https://reviews.llvm.org/D125061.
# Change
This modifies Clang comment parsing so that comments are attached to subsequent
declarations even if there are preprocessor directives between the end of the
comment and the start of the decl. Furthermore, this change:
- Adds tests to verify that comments are attached to their associated decls
even if there are preprocessor directives in between
- Adds tests to verify that current behavior has not changed (i.e. use of the
other characters between comment and decl will result in the comment not being
attached to the decl)
- Updates existing `lit` tests which would otherwise break.
## Complications
Clang [does not yet support](https://github.com/llvm/llvm-project/issues/38206)
attaching doc comments to macros. Consequently, the change proposed in this RFC
affects cases where a doc comment attached to a macro is followed immediately
by a normal declaration. In these cases, the macro's doc comments will be
attached to the subsequent decl. Previously they would be ignored because any
preprocessor directives between a comment and a decl would result in the
comment not being attached to the decl. An example of this is shown below.
```cpp
/// Doc comment for a function-like macro
/// @param n
///A macro argument
#define custom_sqrt(n) __internal_sqrt(n)
int __internal_sqrt(int n) { return __builtin_sqrt(n); }
// NB: the doc comment for the custom_sqrt macro will actually be attached to
__internal_sqrt!
```
There is a real instance of this problem in the Clang codebase, namely here:
https://github.com/llvm/llvm-project/blob/be10070f91b86a6f126d2451852242bfcb2cd366/clang/lib/Headers/amxcomplexintrin.h#L65-L114
As part of this RFC, I've added a semicolon to break up the Clang comment
parsing so that the `-Wdocumentation` errors go away, but this is a hack. The
real solution is to fix Clang comment parsing so that doc comments are properly
attached to macros, however this would be a large change that is outside of the
scope of this RFC.
>From 54c114738a063d18aadedf470f005c3e966bd6e5 Mon Sep 17 00:00:00 2001
From: hdoc
Date: Thu, 11 Apr 2024 01:54:18 -0700
Subject: [PATCH] Attach comments to decl even if preproc directives are in
between
---
clang/lib/AST/ASTContext.cpp | 7 +-
clang/lib/Headers/amxcomplexintrin.h | 4 +
clang/test/Index/annotate-comments.cpp | 5 +-
clang/unittests/AST/DeclTest.cpp | 310 +
4 files changed, 321 insertions(+), 5 deletions(-)
diff --git a/clang/lib/AST/ASTContext.cpp
