Skip to content

[clang-doc] Precommit concept tests #144160

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 and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

evelez7
Copy link
Member

@evelez7 evelez7 commented Jun 13, 2025

No description provided.

@evelez7 evelez7 marked this pull request as ready for review June 13, 2025 21:17
Copy link
Member Author

evelez7 commented Jun 13, 2025

This stack of pull requests is managed by Graphite. Learn more about stacking.

@llvmbot
Copy link
Member

llvmbot commented Jun 13, 2025

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

Author: Erick Velez (evelez7)

Changes

Full diff: https://github.com/llvm/llvm-project/pull/144160.diff

3 Files Affected:

  • (added) clang-tools-extra/test/clang-doc/json/class-requires.cpp (+29)
  • (added) clang-tools-extra/test/clang-doc/json/concept.cpp (+37)
  • (added) clang-tools-extra/test/clang-doc/json/function-requires.cpp (+46)
diff --git a/clang-tools-extra/test/clang-doc/json/class-requires.cpp b/clang-tools-extra/test/clang-doc/json/class-requires.cpp
new file mode 100644
index 0000000000000..022ce94d8c5af
--- /dev/null
+++ b/clang-tools-extra/test/clang-doc/json/class-requires.cpp
@@ -0,0 +1,29 @@
+// RUN: rm -rf %t && mkdir -p %t
+// RUN: clang-doc --extra-arg -std=c++20 --output=%t --format=json --executor=standalone %s
+// RUN: FileCheck %s < %t/GlobalNamespace/MyClass.json
+
+template<typename T>
+concept Addable = requires(T a, T b) {
+  { a + b };
+};
+
+template<typename T>
+requires Addable<T>
+struct MyClass {
+};
+
+// CHECK:       "Name": "MyClass",
+// CHECK-NEXT:  "Namespace": [
+// CHECK-NEXT:    "GlobalNamespace"
+// CHECK-NEXT:  ],
+// CHECK-NEXT:  "Path": "GlobalNamespace",
+// CHECK-NEXT:  "TagType": "struct",
+// CHECK-NEXT:  "Template": {
+// CHECK-NOT:     "Constraints": [
+// CHECK-NOT:       "Addable<T>"
+// CHECK-NOT:     ],
+// CHECK-NEXT:    "Parameters": [
+// CHECK-NEXT:      "typename T"
+// CHECK-NEXT:    ]
+// CHECK-NEXT:  },
+// CHECK-NEXT:  "USR": "{{[A-F0-9]*}}"
diff --git a/clang-tools-extra/test/clang-doc/json/concept.cpp b/clang-tools-extra/test/clang-doc/json/concept.cpp
new file mode 100644
index 0000000000000..78be607d1f2f1
--- /dev/null
+++ b/clang-tools-extra/test/clang-doc/json/concept.cpp
@@ -0,0 +1,37 @@
+// RUN: rm -rf %t && mkdir -p %t
+// RUN: clang-doc --extra-arg -std=c++20 --output=%t --format=json --executor=standalone %s
+
+// Requires that T suports post and pre-incrementing.
+template<typename T>
+concept Incrementable = requires(T x) {
+  ++x;
+  x++;
+};
+
+// CHECK:      {
+// CHECK-NOT:    "Concepts": [
+// CHECK-NOT:      {
+// CHECK-NOT:        "ConstraintExpression": "requires (T x) { ++x; x++; }",
+// CHECK-NOT:        "Description": [
+// CHECK-NOT:        {
+// CHECK-NOT:          "FullComment": {
+// CHECK-NOT:            "Children": [
+// CHECK-NOT:              {
+// CHECK-NOT:                "ParagraphComment": {
+// CHECK-NOT:                  "Children": [
+// CHECK-NOT:                    {
+// CHECK-NOT:                      "TextComment": " Requires that T suports post and pre-incrementing."
+// CHECK-NOT:        },
+// CHECK-NOT:        "IsType": true,
+// CHECK-NOT:        "Name": "Incrementable",
+// CHECK-NOT:        "Template": {
+// CHECK-NOT:          "Parameters": [
+// CHECK-NOT:            "typename T"
+// CHECK-NOT:          ]
+// CHECK-NOT:        },
+// CHECK-NOT:        "USR": "{{[0-9A-F]*}}"
+// CHECK-NOT:      }
+// CHECK-NOT:    ],
+// CHECK:        "Name": "",
+// CHECK:        "USR": "0000000000000000000000000000000000000000"
+// CHECK:      }
diff --git a/clang-tools-extra/test/clang-doc/json/function-requires.cpp b/clang-tools-extra/test/clang-doc/json/function-requires.cpp
new file mode 100644
index 0000000000000..4436ecb879306
--- /dev/null
+++ b/clang-tools-extra/test/clang-doc/json/function-requires.cpp
@@ -0,0 +1,46 @@
+// RUN: rm -rf %t && mkdir -p %t
+// RUN: clang-doc --extra-arg -std=c++20 --output=%t --format=json --executor=standalone %s
+// RUN: FileCheck %s < %t/GlobalNamespace/index.json
+
+template<typename T>
+concept Incrementable = requires(T x) {
+  ++x;
+  x++;
+};
+
+template<typename T> void increment(T t) requires Incrementable<T> {
+  ++t;
+  t++;
+}
+
+// CHECK:       "Functions": [
+// CHECK-NEXT:    {
+// CHECK-NEXT:      "IsStatic": false,
+// CHECK-NEXT:      "Location": {
+// CHECK-NEXT:        "Filename": "{{.*}}function-requires.cpp",
+// CHECK-NEXT:        "LineNumber": 11
+// CHECK-NEXT:      },
+// CHECK-NEXT:      "Name": "increment",
+// CHECK-NEXT:      "Params": [
+// CHECK-NEXT:        {
+// CHECK-NEXT:          "Name": "t",
+// CHECK-NEXT:          "Type": "T"
+// CHECK-NEXT:        }
+// CHECK-NEXT:      ],
+// CHECK-NEXT:      "ReturnType": {
+// CHECK-NEXT:        "IsBuiltIn": false,
+// CHECK-NEXT:        "IsTemplate": false,
+// CHECK-NEXT:        "Name": "void",
+// CHECK-NEXT:        "QualName": "void",
+// CHECK-NEXT:        "USR": "0000000000000000000000000000000000000000"
+// CHECK-NEXT:      },
+// CHECK-NEXT:      "Template": {
+// CHECK-NOT:         "Constraints": [
+// CHECK-NOT:           "Incrementable<T>"
+// CHECK-NOT:         ],
+// CHECK-NEXT:        "Parameters": [
+// CHECK-NEXT:          "typename T"
+// CHECK-NEXT:        ]
+// CHECK-NEXT:      },
+// CHECK-NEXT:      "USR": "{{[0-9A-F]*}}" 
+// CHECK-NEXT:    }

@evelez7 evelez7 requested review from ilovepi and petrhosek June 13, 2025 21:17
// CHECK-NEXT: "TagType": "struct",
// CHECK-NEXT: "Template": {
// CHECK-NOT: "Constraints": [
// CHECK-NOT: "Addable<T>"
Copy link
Member Author

Choose a reason for hiding this comment

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

Interesting thing about Constraints in the context of Clang-Doc is that we don't get back any Decls AFAIK, so I can't construct this as a Reference like BaseRecords. Which is unfortunate in terms of linking. Might be able to find a way to do this but I don't know much about how Expr works.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for pointing this out. As discussed offline, let’s ask clang maintainers on discord or discourse about how to handle this. For this patch things are fine though since it’s just documenting the current behavior.

Copy link
Member Author

Choose a reason for hiding this comment

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

I changed the tests so that the constraints are now arrays of Infos. Implementation is up the stack.

@@ -0,0 +1,37 @@
// RUN: rm -rf %t && mkdir -p %t
// RUN: clang-doc --extra-arg -std=c++20 --output=%t --format=json --executor=standalone %s
Copy link
Member Author

Choose a reason for hiding this comment

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

FileCheck is omitted here because since nothing is mapped, no file is created.

Copy link
Contributor

Choose a reason for hiding this comment

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

Let’s try to mark it fail with filecheck. If that’s doesn’t work, I’m ok with this.

Copy link
Contributor

@ilovepi ilovepi left a comment

Choose a reason for hiding this comment

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

LGTM, modulo some minor comments.

@@ -0,0 +1,37 @@
// RUN: rm -rf %t && mkdir -p %t
// RUN: clang-doc --extra-arg -std=c++20 --output=%t --format=json --executor=standalone %s
Copy link
Contributor

Choose a reason for hiding this comment

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

Let’s try to mark it fail with filecheck. If that’s doesn’t work, I’m ok with this.

// CHECK-NEXT: "TagType": "struct",
// CHECK-NEXT: "Template": {
// CHECK-NOT: "Constraints": [
// CHECK-NOT: "Addable<T>"
Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for pointing this out. As discussed offline, let’s ask clang maintainers on discord or discourse about how to handle this. For this patch things are fine though since it’s just documenting the current behavior.

@evelez7 evelez7 force-pushed the users/evelez7/clang-doc-precommit-concept-tests branch from ad20a26 to 8d8b900 Compare June 16, 2025 21:02
@evelez7 evelez7 requested a review from ilovepi June 16, 2025 21:05
@evelez7 evelez7 force-pushed the users/evelez7/clang-doc-precommit-concept-tests branch from 8d8b900 to b602047 Compare June 16, 2025 21:39
Copy link
Contributor

@ilovepi ilovepi left a comment

Choose a reason for hiding this comment

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

Still LGTM.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants