Skip to content

Conversation

@vbvictor
Copy link
Contributor

@vbvictor vbvictor commented Oct 28, 2025

We use dump_ast_matchers.py script to update this auto-generated file in case someone make changes to ASTMatchers.
This PR ensures that dump_ast_matchers.py was run after change in ASTMatchers.

Closes #165418.

@llvmbot llvmbot added the clang Clang issues not falling into any other category label Oct 28, 2025
@llvmbot
Copy link
Member

llvmbot commented Oct 28, 2025

@llvm/pr-subscribers-clang

Author: Baranov Victor (vbvictor)

Changes

WIP

Closes #165418.


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

2 Files Affected:

  • (modified) clang/include/clang/ASTMatchers/ASTMatchers.h (+1-1)
  • (modified) llvm/utils/git/code-format-helper.py (+64-1)
diff --git a/clang/include/clang/ASTMatchers/ASTMatchers.h b/clang/include/clang/ASTMatchers/ASTMatchers.h
index 98e62de2a9bfb..cc72613db4d15 100644
--- a/clang/include/clang/ASTMatchers/ASTMatchers.h
+++ b/clang/include/clang/ASTMatchers/ASTMatchers.h
@@ -1390,7 +1390,7 @@ extern const internal::VariadicDynCastAllOfMatcher<Expr, RequiresExpr>
 
 /// Matches concept requirement body declaration.
 ///
-/// Example matches '{ *p; }'
+/// Example matches '{ * p; }'
 /// \code
 ///   template<typename T>
 ///   concept dereferencable = requires(T p) { *p; }
diff --git a/llvm/utils/git/code-format-helper.py b/llvm/utils/git/code-format-helper.py
index 406a72817acb8..6de7d21ab68a6 100755
--- a/llvm/utils/git/code-format-helper.py
+++ b/llvm/utils/git/code-format-helper.py
@@ -466,7 +466,70 @@ def format_run(self, changed_files: List[str], args: FormatArgs) -> Optional[str
         return report
 
 
-ALL_FORMATTERS = (DarkerFormatHelper(), ClangFormatHelper(), UndefGetFormatHelper())
+class DumpASTMatchersHelper(FormatHelper):
+    name = "dump_ast_matchers"
+    friendly_name = "AST matchers documentation"
+
+    output_html = "clang/docs/LibASTMatchersReference.html"
+    script_dir = "clang/docs/tools"
+    script_name = "dump_ast_matchers.py"
+
+    @property
+    def instructions(self) -> str:
+        return f"cd {self.script_dir} && python {self.script_name}"
+
+    def should_run(self, changed_files: List[str]) -> List[str]:
+        for file in changed_files:
+            if file == "clang/include/clang/ASTMatchers/ASTMatchers.h":
+                return True
+        return False
+
+    def has_tool(self) -> bool:
+        if not os.path.exists(os.path.join(self.script_dir, self.script_name)):
+            return False
+        return True
+
+    def format_run(self, changed_files: List[str], args: FormatArgs) -> Optional[str]:
+        if not self.should_run(changed_files):
+            return None
+
+        if args.verbose:
+            print(f"Running: {self.instructions}")
+
+        # Run the 'dump_ast_matchers.py' from its directory as specified in the script doc
+        proc = subprocess.run(
+            ["python", self.script_name],
+            stdout=subprocess.PIPE,
+            stderr=subprocess.PIPE,
+            encoding="utf-8",
+            cwd=self.script_dir,
+        )
+
+        if proc.returncode != 0:
+            return f"Execution of 'dump_ast_matchers.py' failed:\n{proc.stderr}\n{proc.stdout}"
+
+        # Check if 'LibASTMatchersReference.html' file was modified
+        cmd = ["git", "diff", "--exit-code", self.output_html]
+        proc = subprocess.run(
+            cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE, encoding="utf-8"
+        )
+
+        # 'LibASTMatchersReference.html' was modified - count as failure
+        if proc.returncode != 0:
+            if args.verbose:
+                print(f"error: {self.name} exited with code {proc.returncode}")
+                print(proc.stdout.decode("utf-8"))
+            return proc.stdout.decode("utf-8")
+        else:
+            return None
+
+
+ALL_FORMATTERS = (
+    DarkerFormatHelper(),
+    ClangFormatHelper(),
+    UndefGetFormatHelper(),
+    DumpASTMatchersHelper(),
+)
 
 
 def hook_main():

@github-actions
Copy link

github-actions bot commented Oct 28, 2025

✅ With the latest revision this PR passed the AST matchers documentation.

@github-actions
Copy link

github-actions bot commented Oct 28, 2025

✅ With the latest revision this PR passed the Python code formatter.

@vbvictor vbvictor force-pushed the acp/vbvictor/7336963445133113 branch from 4b503d5 to 7669f00 Compare October 28, 2025 20:54
@vbvictor vbvictor changed the title [WIP][GitHub][CI] Add running of dump_ast_matchers.py to premerge workflow [GitHub][CI] Add running of 'dump_ast_matchers.py' to premerge workflow Oct 28, 2025
@vbvictor
Copy link
Contributor Author

Adding AST Matchers/clang-tidy maintainers to gather opinion from dump_ast_matchers.py standpoint.

Copy link
Contributor

@boomanaiden154 boomanaiden154 left a comment

Choose a reason for hiding this comment

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

Why do we need to do something in the code formatting action that's completely custom instead of something like #118154?

Also, this should probably just be autogenerated inside the build system.

I tried this for the example above in #113739, but maintainers were not happy.

@vbvictor
Copy link
Contributor Author

Why do we need to do something in the code formatting action that's completely custom instead of something like #118154?

Thank you for reference, I implemented it the easy way. Though formatting message looked cool

The job will fail till we land, #165448.

Copy link
Contributor

@boomanaiden154 boomanaiden154 left a comment

Choose a reason for hiding this comment

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

The PR title and description need to be updated. It would be good if the branch gets updated to kick off CI again too.

@vbvictor vbvictor changed the title [GitHub][CI] Add running of 'dump_ast_matchers.py' to premerge workflow [clang] Add test run of 'dump_ast_matchers.py' Oct 30, 2025
Copy link
Contributor

@boomanaiden154 boomanaiden154 left a comment

Choose a reason for hiding this comment

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

LGTM. Probably would be good to get approval from someone on the clang side as well.

@vbvictor
Copy link
Contributor Author

vbvictor commented Oct 30, 2025

Disabled test on Windows because of error which makes re-generated file different from expected:

<urlopen error [SSL: CERTIFICATE_VERIFY_FAILED] certificate verify failed: unable to get local issuer certificate (_ssl.c:1129)>

This should be either fixed by tinkering with certs in win-builder or by adjusting dump_ast_matchers.py.
But I believe this should be out of current scope because Linux builder still run the test which is platform-independent.

@boomanaiden154
Copy link
Contributor

The test shouldn't be accessing anything over the internet. That should be fixed before landing.

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

Labels

clang Clang issues not falling into any other category

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[CI] Add dump_ast_matchers.py run to code-format CI workflow.

3 participants