[clang-doc] Add regression test for test comments in macros by ZhongUncle · Pull Request #132360 · llvm/llvm-project (original) (raw)

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Conversation19 Commits2 Checks10 Files changed

Conversation

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters

[ Show hidden characters]({{ revealButtonHref }})

ZhongUncle

@ZhongUncle

@ZhongUncle

@github-actions GitHub Actions

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.

You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums.

@llvmbot

@llvm/pr-subscribers-clang

@llvm/pr-subscribers-debuginfo

Author: None (ZhongUncle)

Changes

Patch is 32.17 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/132360.diff

19 Files Affected:

diff --git a/clang-tools-extra/test/clang-doc/macro.cpp b/clang-tools-extra/test/clang-doc/macro.cpp new file mode 100644 index 0000000000000..1b41befadaf2d --- /dev/null +++ b/clang-tools-extra/test/clang-doc/macro.cpp @@ -0,0 +1,32 @@ +// RUN: rm -rf %t && mkdir -p %t +// RUN: clang-doc --format=md --doxygen --output=%t --executor=standalone %s +// RUN: clang-doc --format=html --doxygen --output=%t --executor=standalone %s +// RUN: FileCheck %s < %t/GlobalNamespace/MyClass.md --check-prefix=MD-MyClass-LINE +// RUN: FileCheck %s < %t/GlobalNamespace/MyClass.md --check-prefix=MD-MyClass +// RUN: FileCheck %s < %t/GlobalNamespace/MyClass.html --check-prefix=HTML-MyClass-LINE +// RUN: FileCheck %s < %t/GlobalNamespace/MyClass.html --check-prefix=HTML-MyClass + +#define DECLARE_METHODS \

+}; diff --git a/clang/test/Driver/Inputs/CUDA-symlinks/usr/bin/ptxas b/clang/test/Driver/Inputs/CUDA-symlinks/usr/bin/ptxas deleted file mode 120000 index 59eefd95a9023..0000000000000 --- a/clang/test/Driver/Inputs/CUDA-symlinks/usr/bin/ptxas +++ /dev/null @@ -1 +0,0 @@ -../../opt/cuda/bin/ptxas \ No newline at end of file diff --git a/clang/test/Driver/Inputs/CUDA-symlinks/usr/bin/ptxas b/clang/test/Driver/Inputs/CUDA-symlinks/usr/bin/ptxas new file mode 100644 index 0000000000000..73f4640647c7e --- /dev/null +++ b/clang/test/Driver/Inputs/CUDA-symlinks/usr/bin/ptxas @@ -0,0 +1,5 @@ +XSym +0024 +509d326cf43a00f4244e32ee2f04c3e2 +../../opt/cuda/bin/ptxas

@llvmbot

@llvm/pr-subscribers-clang-tools-extra

Author: None (ZhongUncle)

Changes

Patch is 32.17 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/132360.diff

19 Files Affected:

diff --git a/clang-tools-extra/test/clang-doc/macro.cpp b/clang-tools-extra/test/clang-doc/macro.cpp new file mode 100644 index 0000000000000..1b41befadaf2d --- /dev/null +++ b/clang-tools-extra/test/clang-doc/macro.cpp @@ -0,0 +1,32 @@ +// RUN: rm -rf %t && mkdir -p %t +// RUN: clang-doc --format=md --doxygen --output=%t --executor=standalone %s +// RUN: clang-doc --format=html --doxygen --output=%t --executor=standalone %s +// RUN: FileCheck %s < %t/GlobalNamespace/MyClass.md --check-prefix=MD-MyClass-LINE +// RUN: FileCheck %s < %t/GlobalNamespace/MyClass.md --check-prefix=MD-MyClass +// RUN: FileCheck %s < %t/GlobalNamespace/MyClass.html --check-prefix=HTML-MyClass-LINE +// RUN: FileCheck %s < %t/GlobalNamespace/MyClass.html --check-prefix=HTML-MyClass + +#define DECLARE_METHODS \

+}; diff --git a/clang/test/Driver/Inputs/CUDA-symlinks/usr/bin/ptxas b/clang/test/Driver/Inputs/CUDA-symlinks/usr/bin/ptxas deleted file mode 120000 index 59eefd95a9023..0000000000000 --- a/clang/test/Driver/Inputs/CUDA-symlinks/usr/bin/ptxas +++ /dev/null @@ -1 +0,0 @@ -../../opt/cuda/bin/ptxas \ No newline at end of file diff --git a/clang/test/Driver/Inputs/CUDA-symlinks/usr/bin/ptxas b/clang/test/Driver/Inputs/CUDA-symlinks/usr/bin/ptxas new file mode 100644 index 0000000000000..73f4640647c7e --- /dev/null +++ b/clang/test/Driver/Inputs/CUDA-symlinks/usr/bin/ptxas @@ -0,0 +1,5 @@ +XSym +0024 +509d326cf43a00f4244e32ee2f04c3e2 +../../opt/cuda/bin/ptxas

@llvmbot

@llvm/pr-subscribers-clang-driver

Author: None (ZhongUncle)

Changes

Patch is 32.17 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/132360.diff

19 Files Affected:

diff --git a/clang-tools-extra/test/clang-doc/macro.cpp b/clang-tools-extra/test/clang-doc/macro.cpp new file mode 100644 index 0000000000000..1b41befadaf2d --- /dev/null +++ b/clang-tools-extra/test/clang-doc/macro.cpp @@ -0,0 +1,32 @@ +// RUN: rm -rf %t && mkdir -p %t +// RUN: clang-doc --format=md --doxygen --output=%t --executor=standalone %s +// RUN: clang-doc --format=html --doxygen --output=%t --executor=standalone %s +// RUN: FileCheck %s < %t/GlobalNamespace/MyClass.md --check-prefix=MD-MyClass-LINE +// RUN: FileCheck %s < %t/GlobalNamespace/MyClass.md --check-prefix=MD-MyClass +// RUN: FileCheck %s < %t/GlobalNamespace/MyClass.html --check-prefix=HTML-MyClass-LINE +// RUN: FileCheck %s < %t/GlobalNamespace/MyClass.html --check-prefix=HTML-MyClass + +#define DECLARE_METHODS \

+}; diff --git a/clang/test/Driver/Inputs/CUDA-symlinks/usr/bin/ptxas b/clang/test/Driver/Inputs/CUDA-symlinks/usr/bin/ptxas deleted file mode 120000 index 59eefd95a9023..0000000000000 --- a/clang/test/Driver/Inputs/CUDA-symlinks/usr/bin/ptxas +++ /dev/null @@ -1 +0,0 @@ -../../opt/cuda/bin/ptxas \ No newline at end of file diff --git a/clang/test/Driver/Inputs/CUDA-symlinks/usr/bin/ptxas b/clang/test/Driver/Inputs/CUDA-symlinks/usr/bin/ptxas new file mode 100644 index 0000000000000..73f4640647c7e --- /dev/null +++ b/clang/test/Driver/Inputs/CUDA-symlinks/usr/bin/ptxas @@ -0,0 +1,5 @@ +XSym +0024 +509d326cf43a00f4244e32ee2f04c3e2 +../../opt/cuda/bin/ptxas

@llvmbot

@llvm/pr-subscribers-lldb

Author: None (ZhongUncle)

Changes

Patch is 32.17 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/132360.diff

19 Files Affected:

diff --git a/clang-tools-extra/test/clang-doc/macro.cpp b/clang-tools-extra/test/clang-doc/macro.cpp new file mode 100644 index 0000000000000..1b41befadaf2d --- /dev/null +++ b/clang-tools-extra/test/clang-doc/macro.cpp @@ -0,0 +1,32 @@ +// RUN: rm -rf %t && mkdir -p %t +// RUN: clang-doc --format=md --doxygen --output=%t --executor=standalone %s +// RUN: clang-doc --format=html --doxygen --output=%t --executor=standalone %s +// RUN: FileCheck %s < %t/GlobalNamespace/MyClass.md --check-prefix=MD-MyClass-LINE +// RUN: FileCheck %s < %t/GlobalNamespace/MyClass.md --check-prefix=MD-MyClass +// RUN: FileCheck %s < %t/GlobalNamespace/MyClass.html --check-prefix=HTML-MyClass-LINE +// RUN: FileCheck %s < %t/GlobalNamespace/MyClass.html --check-prefix=HTML-MyClass + +#define DECLARE_METHODS \

+}; diff --git a/clang/test/Driver/Inputs/CUDA-symlinks/usr/bin/ptxas b/clang/test/Driver/Inputs/CUDA-symlinks/usr/bin/ptxas deleted file mode 120000 index 59eefd95a9023..0000000000000 --- a/clang/test/Driver/Inputs/CUDA-symlinks/usr/bin/ptxas +++ /dev/null @@ -1 +0,0 @@ -../../opt/cuda/bin/ptxas \ No newline at end of file diff --git a/clang/test/Driver/Inputs/CUDA-symlinks/usr/bin/ptxas b/clang/test/Driver/Inputs/CUDA-symlinks/usr/bin/ptxas new file mode 100644 index 0000000000000..73f4640647c7e --- /dev/null +++ b/clang/test/Driver/Inputs/CUDA-symlinks/usr/bin/ptxas @@ -0,0 +1,5 @@ +XSym +0024 +509d326cf43a00f4244e32ee2f04c3e2 +../../opt/cuda/bin/ptxas

@ZhongUncle ZhongUncle changed the titleUsers/zhongyijiang/main Add test to clang-doc, it can test comments in macro. Original issue is #59819.

Mar 21, 2025

@ZhongUncle

@ilovepi

ilovepi

return a + b; \
}
// MD-MyClass: ### Add

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

// MD-MyClass: ### Add
// MD-MYCLASS: ### Add

Use all caps for check prefixes
#define DECLARE_METHODS \
/**
* @brief Declare a method to calculate the sum of two numbers

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this comment is in the macro, did you intend it to be?

Also as I understand it the bug wasn't filed about comments within macros, which I seldom see. But about comments attached to macros.

I'd also suggest a simpler macro that doesn't define a full function (which also doesn't appear to actually be part of the macro.).

Maybe the macro could be something like

/// Your doc comments #define ADD(a, b) a+b

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I intend to do it, because I try to restore code in issue. And I test some comment styles in this position, like ///(not work, because new line symbol not work well), /**/(works well) and /**(works, this is also format in issue, but if I use \ in each line end, it will output 2 \ and blank in gnerated markdown).

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also rarely see this kind comment in comments, whether in books or in real projects. But this issue display like this. Maybe I understood this wrongly.

If just regular comment, I think I can write a series test like enum.cpp.

Maybe I'm overcomplicating the problem.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also rarely see this kind comment in comments, whether in books or in real projects. But this issue display like this. Maybe I understood this wrongly.

ah, no, I've confused this with a different issue. Apologies. I should take my own advice not to review code first thing in the morning XD. Lets just make it syntactically correct using trailing \. If the markdown comes out w/ an extra \, we can file a bug for that and handle that separately.

You can confirm what's going on by dumping the AST from clang, or by using some of the debug output from clang-doc.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make sense. I will report something as new issues, or maybe I can write it in my GSoC proposal?

I will fix these files after wake up, because it is already early morning here, and I think I broken up something in the project, so I re-cloned entire project, and even though I used --depth 1, it still takes a very long time when git updating files.

I am very happy to get your help and learn new knowledge on the first day of contacting the project, even though I messed up some things.

// HTML-MyClass:

Declare a method to calculate the sum of two numbers
class MyClass {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is this class testing? It's not clear what property you're trying to exercise. I see you expect the macro to expand in the class, but I don't think you're testing what you think you are.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I didn't use this class, in this style comment, generated content will no this macro, both html and markdown. That why I use a . And again, I try to restore issue example in issue, so I use this style comment.

I also think this class shouldn't be here, but I don't know how to implement a macro function like in issue.

@ilovepi

Also, please change the PR title to

[clang-doc] Add regression test for test comments in macros

The body should contain Fixes #59819. The underlying problem was fixed in https://reviews.llvm.org/D142560, but this patch adds a proper regression test.

ilovepi

@@ -0,0 +1,32 @@
// RUN: rm -rf %t && mkdir -p %t
// RUN: clang-doc --format=md --doxygen --output=%t --executor=standalone %s
// RUN: clang-doc --format=html --doxygen --output=%t --executor=standalone %s

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lets move this down w/ the HTML FileCheck lines so they're grouped together.

I'd also suggest renaming the test file to DR-59819.cpp, which is a common way to indicate that its a regression test (DR stands for Defect Report, and the number is the issue number. I'm fine to leave it with a more readable name, but in that case I'd prefer comments-in-macros.cpp and a comment at the top of the file describing the purpose and properties you're testing for.

@ZhongUncle

I'll take a deeper look at your PR once it only has clang-doc related changes.

This is my problem, sorry, I should put the build in external directory. I cmake a build directory in `llvm-project and I use Mac build it in remote PC. It will create or change some files.

Because build LLVM need a hard device, my Mac is 2018, just 6C6T, too long to build llvm. PC is T5810, E5 2680v4 14C28T and 64GB ram, it can build llvm;clang;lld;clang-tools-extra in 35 min. But I am more familiar with writing code on Mac.

@ZhongUncle ZhongUncle changed the titleAdd test to clang-doc, it can test comments in macro. Original issue is #59819. [clang-doc] Add regression test for test comments in macros

Mar 21, 2025

@ZhongUncle

I am not familiar with PR, so accidentally closed this PR. I create a new one #132510