[clang] [Clang][Comments] Attach comments to decl even if preproc directives are in between (PR #88367)

via cfe-commits cfe-commits at lists.llvm.org
Thu Jun 6 18:45:32 PDT 2024


https://github.com/hdoc updated https://github.com/llvm/llvm-project/pull/88367

>From a98d117c68b33d2e6632a832ff77b0e8258469e6 Mon Sep 17 00:00:00 2001
From: hdoc <github at hdoc.io>
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]), "<input.cc:6:14, col:15>");
   EXPECT_EQ(GetNameInfoRange(Matches[2]), "<input.cc:6:14, col:15>");
 }
+
+TEST(Decl, CommentsAttachedToDecl1) {
+  const SmallVector<StringRef> 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<typename T>
+        #endif
+        void f();
+      )",
+
+      R"(
+        /// Test comment
+
+        #ifdef DOCS
+        template<typename T>
+        #endif
+
+        void f();
+      )",
+  };
+
+  for (const auto code : Sources) {
+    auto AST = tooling::buildASTFromCodeWithArgs(code, /*Args=*/{"-std=c++20"});
+    ASTContext &Ctx = AST->getASTContext();
+
+    auto const *F = selectFirst<FunctionDecl>(
+        "id", match(functionDecl(hasName("f")).bind("id"), Ctx));
+    ASSERT_NE(F, nullptr);
+
+    auto const *C = Ctx.getRawCommentForDeclNoCache(F);
+    ASSERT_NE(C, nullptr);
+    EXPECT_EQ(C->getRawText(Ctx.getSourceManager()), "/// Test comment");
+  }
+}
+
+TEST(Decl, CommentsAttachedToDecl2) {
+  const SmallVector<StringRef> 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<typename T>
+        #endif
+        void f();
+      )",
+
+      R"(
+        /** Test comment
+         */
+
+        #ifdef DOCS
+        template<typename T>
+        #endif
+
+        void f();
+      )",
+  };
+
+  for (const auto code : Sources) {
+    auto AST = tooling::buildASTFromCodeWithArgs(code, /*Args=*/{"-std=c++20"});
+    ASTContext &Ctx = AST->getASTContext();
+
+    auto const *F = selectFirst<FunctionDecl>(
+        "id", match(functionDecl(hasName("f")).bind("id"), Ctx));
+    ASSERT_NE(F, nullptr);
+
+    auto const *C = Ctx.getRawCommentForDeclNoCache(F);
+    ASSERT_NE(C, nullptr);
+    EXPECT_EQ(C->getRawText(Ctx.getSourceManager()),
+              "/** Test comment\n         */");
+  }
+}
+
+TEST(Decl, CommentsAttachedToDecl3) {
+  const SmallVector<StringRef> Sources{
+      R"(
+        /// @brief Test comment
+        void f();
+      )",
+
+      R"(
+        /// @brief Test comment
+
+        void f();
+      )",
+
+      R"(
+        /// @brief Test comment
+        #if 0
+        // tralala
+        #endif
+        void f();
+      )",
+
+      R"(
+        /// @brief Test comment
+
+        #if 0
+        // tralala
+        #endif
+
+        void f();
+      )",
+
+      R"(
+        /// @brief Test comment
+        #ifdef DOCS
+        template<typename T>
+        #endif
+        void f();
+      )",
+
+      R"(
+        /// @brief Test comment
+
+        #ifdef DOCS
+        template<typename T>
+        #endif
+
+        void f();
+     )",
+  };
+
+  for (const auto code : Sources) {
+    auto AST = tooling::buildASTFromCodeWithArgs(code, /*Args=*/{"-std=c++20"});
+    ASTContext &Ctx = AST->getASTContext();
+
+    auto const *F = selectFirst<FunctionDecl>(
+        "id", match(functionDecl(hasName("f")).bind("id"), Ctx));
+    ASSERT_NE(F, nullptr);
+
+    auto const *C = Ctx.getRawCommentForDeclNoCache(F);
+    ASSERT_NE(C, nullptr);
+    EXPECT_EQ(C->getRawText(Ctx.getSourceManager()), "/// @brief Test comment");
+  }
+}
+
+TEST(Decl, CommentsAttachedToDecl4) {
+  const SmallVector<StringRef> Sources{
+      R"(
+        /** \brief Test comment
+         */
+        void f();
+      )",
+
+      R"(
+        /** \brief Test comment
+         */
+
+        void f();
+      )",
+
+      R"(
+        /** \brief Test comment
+         */
+        #if 0
+        /* tralala */
+        #endif
+        void f();
+      )",
+
+      R"(
+        /** \brief Test comment
+         */
+
+        #if 0
+        /* tralala */
+        #endif
+
+        void f();
+      )",
+
+      R"(
+        /** \brief Test comment
+         */
+        #ifdef DOCS
+        template<typename T>
+        #endif
+        void f();
+      )",
+
+      R"(
+        /** \brief Test comment
+         */
+
+        #ifdef DOCS
+        template<typename T>
+        #endif
+
+        void f();
+     )",
+  };
+
+  for (const auto code : Sources) {
+    auto AST = tooling::buildASTFromCodeWithArgs(code, /*Args=*/{"-std=c++20"});
+    ASTContext &Ctx = AST->getASTContext();
+
+    auto const *F = selectFirst<FunctionDecl>(
+        "id", match(functionDecl(hasName("f")).bind("id"), Ctx));
+    ASSERT_NE(F, nullptr);
+
+    auto const *C = Ctx.getRawCommentForDeclNoCache(F);
+    ASSERT_NE(C, nullptr);
+    EXPECT_EQ(C->getRawText(Ctx.getSourceManager()),
+              "/** \\brief Test comment\n         */");
+  }
+}
+
+/// This example intentionally inserts characters between a doc comment and the
+/// associated declaration to verify that the comment does not become associated
+/// with the FunctionDecl.
+/// By default, Clang does not allow for other declarations (aside from
+/// preprocessor directives, as shown above) to be placed between a doc comment
+/// and a declaration.
+TEST(Decl, CommentsAttachedToDecl5) {
+  const SmallVector<StringRef> Sources{
+      R"(
+        /// Test comment
+        ;
+        void f();
+      )",
+
+      R"(
+        /// Test comment
+        // @
+        void f();
+      )",
+
+      R"(
+        /// Test comment
+        // {}
+        void f();
+      )",
+  };
+
+  for (const auto code : Sources) {
+    auto AST = tooling::buildASTFromCodeWithArgs(code, /*Args=*/{"-std=c++20"});
+    ASTContext &Ctx = AST->getASTContext();
+
+    auto const *F = selectFirst<FunctionDecl>(
+        "id", match(functionDecl(hasName("f")).bind("id"), Ctx));
+    ASSERT_NE(F, nullptr);
+
+    auto const *C = Ctx.getRawCommentForDeclNoCache(F);
+    ASSERT_EQ(C, nullptr);
+  }
+}

>From 4e07b18009e7e6d60eb96f21909a408fa635f33b Mon Sep 17 00:00:00 2001
From: hdoc <github at hdoc.io>
Date: Sat, 4 May 2024 19:09:30 -0700
Subject: [PATCH 2/3] Document internal version of function to fix lit error

The lit test compiles the code with -Wdocumentation set, so we need to
document the internal function so that -Wdocumentation doesn't throw
errors
---
 clang/lib/Headers/amxcomplexintrin.h | 22 ++++++++++++++++++----
 1 file changed, 18 insertions(+), 4 deletions(-)

diff --git a/clang/lib/Headers/amxcomplexintrin.h b/clang/lib/Headers/amxcomplexintrin.h
index 19eaee10ede65..6ae40d2eab5c8 100644
--- a/clang/lib/Headers/amxcomplexintrin.h
+++ b/clang/lib/Headers/amxcomplexintrin.h
@@ -62,8 +62,6 @@
 ///    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
@@ -109,8 +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
+///    accumulate the results into a packed single precision tile.
+///
+/// \param m
+///    The number of rows in the first tile and the number of rows in the result
+///    tile.
+/// \param n
+///    The number of columns in the second tile and the number of columns in the
+///    result tile.
+/// \param k
+///    The number of columns in the first tile and the number of rows in the
+///    second tile.
+/// \param dst
+///    Pointer to the destination tile where the result will be stored.
+/// \param src1
+///    Pointer to the first source tile.
+/// \param src2
+///    Pointer to the second source tile.
 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) {

>From 571fb55f9e1ab33d78d0941edbad86a988390fbe Mon Sep 17 00:00:00 2001
From: hdoc <github at hdoc.io>
Date: Thu, 6 Jun 2024 18:44:16 -0700
Subject: [PATCH 3/3] Add comment to _wbinvd function to avoid -Wdocumentation
 error

---
 clang/lib/Headers/ia32intrin.h | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/clang/lib/Headers/ia32intrin.h b/clang/lib/Headers/ia32intrin.h
index c9c92b9ee0f99..1cdc53584a1f5 100644
--- a/clang/lib/Headers/ia32intrin.h
+++ b/clang/lib/Headers/ia32intrin.h
@@ -533,8 +533,13 @@ __rdtscp(unsigned int *__A) {
 /// \see __rdpmc
 #define _rdpmc(A) __rdpmc(A)
 
-;
-
+/// Invalidates the contents of the processor's internal caches.
+///    This function writes back and invalidates all modified cache lines in
+///    the processor.
+///
+/// \headerfile <x86intrin.h>
+///
+/// This intrinsic corresponds to the \c WBINVD instruction.
 static __inline__ void __DEFAULT_FN_ATTRS
 _wbinvd(void) {
   __builtin_ia32_wbinvd();



More information about the cfe-commits mailing list