[clang-tools-extra] Update clang tidy Contributing guide (PR #106672)

via cfe-commits cfe-commits at lists.llvm.org
Thu Aug 29 23:44:57 PDT 2024 

https://github.com/MichelleCDjunaidi created https://github.com/llvm/llvm-project/pull/106672

Update the documentation to direct new users to the Github instead of the discontinued Phabricator archive. Also details more ways and information regarding clang-query usage. Partially resolves/disclaims #106656 and #106663 as per discussion in https://discourse.llvm.org/t/inconsistency-between-hasdescendant-in-clang-query-and-clang-libtooling-matchers/80799/.

Also updates the out-of-tree guide.

For context, I recently went through the Contributing guide while writing https://github.com/llvm/llvm-project/pull/102299, and many of these updates were from my experience trying to follow the guide. e.g. I was trying to link the shared library of an out-of-tree check as SHARED in CMake and encountered duplicate symbols like _ZTIN5clang4tidy14ClangTidyCheckE. It wasn't until I saw  https://github.com/llvm/llvm-project/commit/84f137a590e7de25c4105303e5938c40566c2dfb that I found out I had to use MODULE. I also encountered the clang-query difference which was a surprise as the documentation said the two matchers were "virtually identical". Also, the -header-filter thing tripped me out until I found https://github.com/llvm/llvm-project/issues/25590 and https://github.com/llvm/llvm-project/pull/91400. Usually, when people say restrict and filter, they mean filter out (since -header-filter instead includes/filters in said headers).

>From 3518ede871664d3d0f45a51b9e462b53ffa4a7d4 Mon Sep 17 00:00:00 2001
From: MichelleCDjunaidi <87893361+MichelleCDjunaidi at users.noreply.github.com>
Date: Fri, 30 Aug 2024 14:40:39 +0800
Subject: [PATCH 1/3] update outdated information like directing to Phabricator

---
 clang-tools-extra/docs/clang-tidy/Contributing.rst | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/clang-tools-extra/docs/clang-tidy/Contributing.rst b/clang-tools-extra/docs/clang-tidy/Contributing.rst
index 92074bd4dae8ba..176a112d3013ed 100644
--- a/clang-tools-extra/docs/clang-tidy/Contributing.rst
+++ b/clang-tools-extra/docs/clang-tidy/Contributing.rst
@@ -130,11 +130,12 @@ So you have an idea of a useful check for :program:`clang-tidy`.
 First, if you're not familiar with LLVM development, read through the `Getting
 Started with LLVM`_ document for instructions on setting up your workflow and
 the `LLVM Coding Standards`_ document to familiarize yourself with the coding
-style used in the project. For code reviews we mostly use `LLVM Phabricator`_.
+style used in the project. For code reviews we currently use `LLVM Github`_,
+though historically we used Phabricator.
 
 .. _Getting Started with LLVM: https://llvm.org/docs/GettingStarted.html
 .. _LLVM Coding Standards: https://llvm.org/docs/CodingStandards.html
-.. _LLVM Phabricator: https://llvm.org/docs/Phabricator.html
+.. _LLVM Github: https://github.com/llvm/llvm-project
 
 Next, you need to decide which module the check belongs to. Modules
 are located in subdirectories of `clang-tidy/
@@ -690,8 +691,8 @@ warnings and errors. The script provides multiple configuration flags.
   that the file names are matched against.
   ``run-clang-tidy.py clang-tidy/.*Check\.cpp`` will only analyze clang-tidy
   checks. It may also be necessary to restrict the header files that warnings
-  are displayed from using the ``-header-filter`` flag. It has the same behavior
-  as the corresponding :program:`clang-tidy` flag.
+  are displayed from using the ``-exclude-header-filter`` flag. It has the same 
+  behavior as the corresponding :program:`clang-tidy` flag.
 
 * To apply suggested fixes ``-fix`` can be passed as an argument. This gathers
   all changes in a temporary directory and applies them. Passing ``-format``
@@ -758,4 +759,4 @@ There is only one argument that controls profile storage:
 
   * If you run :program:`clang-tidy` from within ``/foo`` directory, and specify
     ``-store-check-profile=.``, then the profile will still be saved to
-    ``/foo/<ISO8601-like timestamp>-example.cpp.json``
+    ``/foo/<ISO8601-like timestamp>-example.cpp.json``
\ No newline at end of file

>From 70c9c6a154ec9df2114f140b827e8b30d59eda2f Mon Sep 17 00:00:00 2001
From: MichelleCDjunaidi <87893361+MichelleCDjunaidi at users.noreply.github.com>
Date: Fri, 30 Aug 2024 14:41:49 +0800
Subject: [PATCH 2/3] update clang-query section to be more detailed

Also disclaims the mismatch between clang-query and clang-tidy as discussed here https://discourse.llvm.org/t/inconsistency-between-hasdescendant-in-clang-query-and-clang-libtooling-matchers/80799/
---
 .../docs/clang-tidy/Contributing.rst          | 25 +++++++++++++------
 1 file changed, 18 insertions(+), 7 deletions(-)

diff --git a/clang-tools-extra/docs/clang-tidy/Contributing.rst b/clang-tools-extra/docs/clang-tidy/Contributing.rst
index 176a112d3013ed..7bb6c116259f68 100644
--- a/clang-tools-extra/docs/clang-tidy/Contributing.rst
+++ b/clang-tools-extra/docs/clang-tidy/Contributing.rst
@@ -337,13 +337,24 @@ a starting point for your test cases.  A rough outline of the process looks like
 The quickest way to prototype your matcher is to use :program:`clang-query` to
 interactively build up your matcher.  For complicated matchers, build up a matching
 expression incrementally and use :program:`clang-query`'s ``let`` command to save named
-matching expressions to simplify your matcher.  Just like breaking up a huge function
-into smaller chunks with intention-revealing names can help you understand a complex
-algorithm, breaking up a matcher into smaller matchers with intention-revealing names
-can help you understand a complicated matcher.  Once you have a working matcher, the
-C++ API will be virtually identical to your interactively constructed matcher.  You can
-use local variables to preserve your intention-revealing names that you applied to
-nested matchers.
+matching expressions to simplify your matcher.
+
+.. code-block:: console
+  clang-query> let c1 cxxRecordDecl()
+  clang-query> match c1
+
+Alternatively, pressing tab after a previous matcher's open brace would also show which 
+matchers can be chained with the previous matcher, though some matchers that work may not 
+be listed.
+
+Just like breaking up a huge function into smaller chunks with intention-revealing names 
+can help you understand a complex algorithm, breaking up a matcher into smaller matchers 
+with intention-revealing names can help you understand a complicated matcher.  
+
+Once you have a working clang-query matcher, the C++ API will be the same or similar to your 
+interactively constructed matcher (there will be cases where they differ slightly). 
+You can use local variables to preserve your intention-revealing names that you applied 
+to nested matchers.
 
 Creating private matchers
 ^^^^^^^^^^^^^^^^^^^^^^^^^

>From 88b1013260433fe05929c5796f3e67eea787b9b8 Mon Sep 17 00:00:00 2001
From: MichelleCDjunaidi <87893361+MichelleCDjunaidi at users.noreply.github.com>
Date: Fri, 30 Aug 2024 14:43:23 +0800
Subject: [PATCH 3/3] update out-of-tree guide

---
 .../docs/clang-tidy/Contributing.rst           | 18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)

diff --git a/clang-tools-extra/docs/clang-tidy/Contributing.rst b/clang-tools-extra/docs/clang-tidy/Contributing.rst
index 7bb6c116259f68..e92e86621c684d 100644
--- a/clang-tools-extra/docs/clang-tidy/Contributing.rst
+++ b/clang-tools-extra/docs/clang-tidy/Contributing.rst
@@ -658,10 +658,13 @@ directory.  The path to this directory is available in a lit test with the varia
 Out-of-tree check plugins
 -------------------------
 
+
 Developing an out-of-tree check as a plugin largely follows the steps
-outlined above. The plugin is a shared library whose code lives outside
+outlined above, including creating a new module and doing the hacks to 
+register the module. The plugin is a shared library whose code lives outside
 the clang-tidy build system. Build and link this shared library against
-LLVM as done for other kinds of Clang plugins.
+LLVM as done for other kinds of Clang plugins. If using CMake, use the keyword
+MODULE while invoking add_library or llvm_add_library.
 
 The plugin can be loaded by passing `-load` to `clang-tidy` in addition to the
 names of the checks to enable.
@@ -676,6 +679,17 @@ compiled against the version of clang-tidy that will be loading the plugin.
 The plugins can use threads, TLS, or any other facilities available to in-tree
 code which is accessible from the external headers.
 
+Note that testing checks out of tree might involve getting ``llvm-lit`` from an 
+installed version of LLVM through the `Stand-alone Builds`_ section. Alternatively,
+get `lit`_ following the `test-suite guide`_ and get the `FileCheck`_ binary, and
+write a version of `check_clang_tidy.py`_ to suit your needs.
+
+.. _Stand-alone Builds: https://llvm.org/docs/GettingStarted.html
+.. _test-suite guide: https://llvm.org/docs/TestSuiteGuide.html
+.. _lit: https://llvm.org/docs/CommandGuide/lit.html
+.. _FileCheck: https://llvm.org/docs/CommandGuide/FileCheck.html
+.. _check_clang_tidy.py: https://github.com/llvm/llvm-project/blob/main/clang-tools-extra/test/clang-tidy/check_clang_tidy.py
+
 Running clang-tidy on LLVM
 --------------------------

More information about the cfe-commits mailing list