Skip to content

Commit

Permalink
Fix clang-tidy plugin compilation on windows and document the process.
Browse files Browse the repository at this point in the history
  • Loading branch information
moxian committed Feb 6, 2025
1 parent 33c22b3 commit e31b049
Show file tree
Hide file tree
Showing 9 changed files with 276 additions and 15 deletions.
236 changes: 236 additions & 0 deletions doc/c++/DEVELOPER_TOOLING.md
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,242 @@ lit -v build/tools/clang-tidy-plugin/test

#### Windows

If you just want to run the default clang-tidy checks, and skip the cata-specific ones, the easiest way is probably to install Clang Power Tools visual studio extension (in the `Extensions` > `Manage extensions..` menu).

Otherwise it is probably faster and easier to install WSL and follow the steps described above in [Extreme tl;dr for Ubuntu Focal (including WSL)](<#extreme-tldr-for-ubuntu-focal-including-wsl>). However building on windows is still possible.

You will need:
- windows powershell console
- Git
- CMake
- Python
- A working C++ compiler that can be run from the windows console, such as MSBuild (aka "Visual Studio Build Tools"; it's part of Visual Studio, but can also be obtained without VS from [this link](https://visualstudio.microsoft.com/downloads/?q=build+tools) (scroll down)) or standalone clang or gcc
- notably, you *can't* use the one provided by msys2/mingw for this (or, well, you can, but you are on your own then)
- (optionall) specifically clang compiler. Can often be downloaded from llvm github https://github.com/llvm/llvm-project/releases . If you don't have one, you can build clang from source with the instructions below
- Ninja build system
- (for running tests) GNU core utilities or an msys2 install (specifically we just need the `diff` tool).
- vcpkg

There are compromises, and you can make do with a different set of tools, but these are what this doc will use.

The rough outline of the process is:
- Obtain a working set of LLVM libraries
- Build clang-tidy with custom checks
- (optional) Test that the checks work
- Run the custom clang-tidy

##### Build LLVM

First of all you would need a working LLVM. You *might* be able to [download it](https://github.com/llvm/llvm-project/releases) from llvm-project github (look for file named something like `clang+llvm-19.1.5-x86_64-pc-windows-msvc.tar.xz`), and it *might* work, but there is no guarantee. I suggest you try it, and skip to the next section. If it works for you - great! You just saved yourself several hours of building it from source, if not - read on.

Ideally you should try to match the llvm version you download with the one that we build the CI against, as different versions yield slightly different results. And also there is no stability promise so that the checks that compile against llvm-17 might stop doing so in llvm-20.
At the time of writing we are building against llvm-17, if you are reading this in the future, check [the CI definition](https://github.com/CleverRaven/Cataclysm-DDA/blob/master/.github/workflows/clang-tidy.yml) to know what's current.


The instruction for building LLVM can be found at https://clang.llvm.org/get_started.html, but duplicating here with some emphasis.

Clone the LLVM repo from https://github.com/llvm/llvm-project.git and checkout the appropriate branch.
The LLVM repo is pretty big so it's recommended to fetch just the commit you want to build at:

```ps1
git clone --depth=1 --branch=release/17.x https://github.com/llvm/llvm-project.git
```

The first step to build the code is to run CMake to generate the makefile.
On the root dir of LLVM, run the following script.
Make sure CMake and python are on the path.

```ps1
# for visual studio
mkdir -p build
cd build
cmake `
-G "Visual Studio 16 2019" `
-DLLVM_ENABLE_PROJECTS='clang;clang-tools-extra;lld' `
-DLLVM_TARGETS_TO_BUILD='X86' `
../llvm
# for non-visual studio
mkdir -p build
cd build
cmake `
-G "Ninja" `
-DLLVM_ENABLE_PROJECTS='clang;clang-tools-extra;lld' `
-DCMAKE_BUILD_TYPE=RelWithDebInfo `
-DLLVM_TARGETS_TO_BUILD='X86' `
-DCMAKE_EXPORT_COMPILE_COMMANDS=ON `
-DLLVM_HOST_TRIPLE=x86_64 `
../llvm
```

If you're using a non-msvc compiler you either need its containing folder on your `PATH`, specify said containing folder in `-DCMAKE_PROGRAM_PATH=<your/path/here>` or specify the full path to the binary in `-DCMAKE_C_COMPILER` as the error message should suggest. Same goes for ninja and python.


Now that we have the build files, we can actually build it by running:

```ps1
# for Visual Studio / MSBuild
cmake --build . --target=clang-tidy --target=clangTidyMain --target=FileCheck --config=RelWithDebInfo
# for ninja
cmake --build . --target=clang-tidy --target=clangTidyMain --target=FileCheck --parallel 4
```

If you don't have clang already, add a `--target=clang --target=llvm-rc --target=lld --target=llvm-objdump` to the command line above.
Remove `--target=FileCheck` if you don't intend to test custom checks.
LLVM is a behemoth so it will take several hours to finish while using 100% of your CPU, even when building only just clang-tidy. Consider stepping out and doing something productive while it's running. (On my machine it took 2h30m to build clang-tidy itself, and 2h to build the other tools afterwards).

Now, if you wanted to make things super smooth going forward, you could build all the targets (not just the select few) and then bundle the built llvm to some nice place via
```ps1
cmake --build . --config=RelWithDebInfo # no target specification
cmake --install --prefix=<some-path-here> .
```
That *might* even only add an hour or so to the total time? Your call whether it's worth it.


##### Build clang-tidy with custom checks

After building clang-tidy as a library from the LLVM source, the next step is to
build clang-tidy as an executable, with the custom checks from the CDDA source.

We will need to run a cmake with lots of arguments, and it gets unwieldy on the command line. So create `CMakeUserPresets.json` file at the root of cataclysm project with the following contents, replacing the paths in `<>` as appropriate:

```json
{
"version": 2,
"cmakeMinimumRequired": {
"major": 3,
"minor": 20,
"patch": 0
},
"configurePresets": [
{
"name": "cata-clang-tidy",
"generator": "Ninja",
"binaryDir": "${sourceDir}/out/build/${presetName}/",
"environment":{
"MY_LLVM_INSTALL_DIR": "<C:/path/to/installed-llvm>",
"VCPKG_ROOT": "<C:/path/to/your/vcpkg>"
},
"cacheVariables": {
"CATA_CLANG_TIDY_INCLUDE_DIR": "<C:/path/to/llvm-source>/clang-tools-extra/",
"CMAKE_PROGRAM_PATH": "<see-below>",


"LLVM_DIR": "$env{MY_LLVM_INSTALL_DIR}/lib/cmake/llvm",
"Clang_DIR": "$env{MY_LLVM_INSTALL_DIR}/lib/cmake/clang",
"CMAKE_BUILD_TYPE": "RelWithDebInfo",
"CATA_CHECK_CLANG_TIDY": "python ${sourceDir}/tools/clang-tidy-plugin/test/check_clang_tidy.py -clang-tidy-binary=${sourceDir}/out/build/${presetName}/tools/clang-tidy-plugin/CataAnalyzer.exe",
"CMAKE_EXPORT_COMPILE_COMMANDS": true,
"CATA_CLANG_TIDY_EXECUTABLE": true,

"VCPKG_MANIFEST_DIR": "${sourceDir}/msvc-full-features",
"CMAKE_TOOLCHAIN_FILE": "$env{VCPKG_ROOT}/scripts/buildsystems/vcpkg.cmake",
"TILES": true, "SOUND": true, "BACKTRACE": true,
"LOCALIZE": true,
"GETTEXT_MSGFMT_BINARY": "cat"
}
}
]
}
```

If you downloaded the packaged llvm archive, or `install`ed it yourself, then `<C:/path/to/installed-llvm>` would point to that. Otherwise it would be `<C:/path/to/llvm-source>/build`.
If you have a packaged version, you can omit `"Clang_DIR"` and `"CATA_CLANG_TIDY_INCLUDE_DIR"`
If you don't intend to run tests in the next step - skip `"CATA_CHECK_CLANG_TIDY"`.
Adjust `"TILES"`, `"SOUND"`, `"BACKTRACE"` and `"LOCALIZE"` as you would for building the game normally, although it is strongly suggested to keep `"LOCALIZE": true` to avoid excessive translation-related false-positives.
`CMAKE_PROGRAM_PATH` should contain the list of directories that hold compiler binaries that are not already in your PATH env variable. I.e. if you have a packaged llvm, set `CMAKE_PROGRAM_PATH` to `<C:/path/to/installed-llvm>/bin/`; otherwise set it to `<C:/path/to/llvm-source>/build/RelWithDebInfo/bin/`. You can add Ninja and python to this list (semicolon-separated).


Run cmake to generate build files under `.\out\build\cata-clang-tidy\`:
```ps1
cmake --preset=cata-clang-tidy
```
(If you adjust the configuration, and then rerun the command but the changes don't seem to be picked up, add a `--fresh` argument)

And then build it
```ps1
cmake --build .\out\build\cata-clang-tidy\ --target=CataAnalyzer
```

Finally, confirm that you do, in fact, have the cata-specific checks:
```ps1
> .\out\build\cata-clang-tidy\tools\clang-tidy-plugin\CataAnalyzer.exe '--checks=-*,cata-*' --list-checks
Enabled checks:
cata-almost-never-auto
cata-assert
cata-avoid-alternative-tokens
cata-combine-locals-into-point
cata-determinism
cata-header-guard
cata-json-translation-input
cata-large-inline-function
cata-large-stack-object
cata-no-long
cata-no-static-translation
cata-ot-match
cata-point-initialization
cata-redundant-parentheses
cata-serialize
cata-simplify-point-constructors
cata-static-declarations
cata-static-initialization-order
cata-static-int_id-constants
cata-static-string_id-constants
cata-test-filename
cata-tests-must-restore-global-state
cata-text-style
cata-translate-string-literal
cata-translations-in-debug-messages
cata-u8-path
cata-unit-overflow
cata-unsequenced-calls
cata-unused-statics
cata-use-localized-sorting
cata-use-mdarray
cata-use-named-point-constants
cata-use-point-apis
cata-use-point-arithmetic
cata-use-string_view
cata-utf8-no-to-lower-to-upper
cata-xy
```

##### (Optional) Test custom checks work

LLVM folks seem to never excercise their test driver in windows console, so it has been broken since 2022 and needs a small patch to work.

```ps1
cd <path-to-llvm-source>
git apply -v --ignore-whitespace <path-to-cata-source>\tools\clang-tidy-plugin\lit-win-console-llvm-17.patch
```

You will need to have `FileCheck` (distributed with LLVM or self-built in the above step) and `diff` (either downloaded as part of [GnuWin32 tools](https://getgnuwin32.sourceforge.net/), or if you have msys2 installed then you can find it in `<path-to-msys2>\usr\bin\`) in your PATH:
```ps1
$env:PATH += ';<C:\path\to\installed-llvm>\bin\;<C:\msys2>\usr\bin'
```
Note the leading `;`

Once you have that, run the test suite
```ps1
cd <path-to-cata-source>
python <path-to-llvm-source>\llvm\utils\lit\lit.py -v .\out\build\cata-clang-tidy\tools\clang-tidy-plugin\test
```

#### Run custom-built clang-tidy on your code

Now you should have a clang-tidy binary at `.\out\build\cata-clang-tidy\tools\clang-tidy-plugin\CataAnalyzer.exe` (which you can move to a more convenient place)
and (as long as you passed `CMAKE_EXPORT_COMPILE_COMMANDS`) a compilation database at `.\out\build\cata-clang-tidy\compile_commands.json`

You can now run clang-tidy on a file of your choosing as
```ps1
CataAnalyzer.exe -p .\out\build\cata-clang-tidy\ --header-filter='nothing' src\ascii_art.cpp
```


#### Windows MinGW-w64

(This section appears to be out of date, but is preserved for historical reasons, in case someone wishes to revive it)

##### Build LLVM

It is probably faster and easier to install WSL and follow the steps described above in [Extreme tl;dr for Ubuntu Focal (including WSL)](<#extreme-tldr-for-ubuntu-focal-including-wsl>).
Expand Down
6 changes: 5 additions & 1 deletion tools/clang-tidy-plugin/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ include(ExternalProject)
find_package(LLVM REQUIRED CONFIG)
find_package(Clang REQUIRED CONFIG)

set(CMAKE_CXX_STANDARD 17)

set(CataAnalyzerSrc
AlmostNeverAutoCheck.cpp
AssertCheck.cpp
Expand Down Expand Up @@ -45,7 +47,8 @@ set(CataAnalyzerSrc
UseStringViewCheck.cpp
UTF8ToLowerUpperCheck.cpp
Utils.cpp
XYCheck.cpp)
XYCheck.cpp
)

if (CATA_CLANG_TIDY_EXECUTABLE)
set(CataAnalyzerName CataAnalyzer)
Expand All @@ -55,6 +58,7 @@ if (CATA_CLANG_TIDY_EXECUTABLE)
else ()
set(CataAnalyzerName CataAnalyzerPlugin)
add_library(${CataAnalyzerName} MODULE ${CataAnalyzerSrc})
target_link_libraries(${CataAnalyzerName} PRIVATE clangTidyPlugin)
if ("${CMAKE_SYSTEM_NAME}" MATCHES "Darwin")
target_link_options(${CataAnalyzerName} PRIVATE -undefined dynamic_lookup)
endif ()
Expand Down
6 changes: 3 additions & 3 deletions tools/clang-tidy-plugin/CataTidyModule.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -59,10 +59,10 @@ class CataModule : public ClangTidyModule
// the same version we linked against

std::string RuntimeVersion = getClangFullVersion();
if( !StringRef( RuntimeVersion ).contains( "clang version " CLANG_VERSION_STRING ) ) {
if( !llvm::StringRef( RuntimeVersion ).contains( "clang version " CLANG_VERSION_STRING ) ) {
llvm::report_fatal_error(
Twine( "clang version mismatch in CataTidyModule. Compiled against "
CLANG_VERSION_STRING " but loaded by ", RuntimeVersion ) );
llvm::Twine( "clang version mismatch in CataTidyModule. Compiled against "
CLANG_VERSION_STRING " but loaded by ", RuntimeVersion ) );
abort(); // NOLINT(cata-assert)
}
CheckFactories.registerCheck<AlmostNeverAutoCheck>( "cata-almost-never-auto" );
Expand Down
3 changes: 0 additions & 3 deletions tools/clang-tidy-plugin/TranslatorCommentsCheck.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,6 @@ namespace clang
{
namespace ast_matchers
{
namespace
{
AST_POLYMORPHIC_MATCHER_P2( hasImmediateArgument,
AST_POLYMORPHIC_SUPPORTED_TYPES( CallExpr, CXXConstructExpr ),
unsigned int, N, ast_matchers::internal::Matcher<Expr>, InnerMatcher )
Expand All @@ -53,7 +51,6 @@ AST_MATCHER_P( StringLiteral, isMarkedString, tidy::cata::TranslatorCommentsChec
return Check->MarkedStrings.find( Loc ) != Check->MarkedStrings.end();
static_cast<void>( Builder );
}
} // namespace
} // namespace ast_matchers
namespace tidy::cata
{
Expand Down
24 changes: 24 additions & 0 deletions tools/clang-tidy-plugin/lit-win-console-llvm-17.patch
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
index cd803f859..638313405 100644
--- a/llvm/utils/lit/lit/TestRunner.py
+++ b/llvm/utils/lit/lit/TestRunner.py
@@ -1085,7 +1085,7 @@ def executeScript(test, litConfig, tmpBase, commands, cwd):
if match:
command = match.group(2)
commands[i] = match.expand(
- "echo '\\1' > nul && " if command else "echo '\\1' > nul"
+ "echo '\\1' > nul && \\2" if command else "echo '\\1' > nul"
)
if litConfig.echo_all_commands:
f.write("@echo on\n")
diff --git a/llvm/utils/lit/lit/TestingConfig.py b/llvm/utils/lit/lit/TestingConfig.py
index 76fd66502..129a5df28 100644
--- a/llvm/utils/lit/lit/TestingConfig.py
+++ b/llvm/utils/lit/lit/TestingConfig.py
@@ -71,6 +71,7 @@ class TestingConfig(object):
"INCLUDE",
"LIB",
"PATHEXT",
+ "SystemDrive",
"USERPROFILE",
]
environment["PYTHONBUFFERED"] = "1"
2 changes: 1 addition & 1 deletion tools/clang-tidy-plugin/test/almost-never-auto.cpp
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// RUN: %check_clang_tidy %s cata-almost-never-auto %t -- --load=%cata_plugin --
// RUN: %check_clang_tidy %s cata-almost-never-auto %t -- --load=%cata_plugin -- -fno-delayed-template-parsing

using int_alias = int;
int_alias return_int_alias();
Expand Down
2 changes: 1 addition & 1 deletion tools/clang-tidy-plugin/test/lit.cfg
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ cata_include = os.path.join( config.cata_source_dir, "./src" )
cata_third_party_include = os.path.join( config.cata_source_dir, "./src/third-party" )
test_include = os.path.join( config.cata_source_dir, "./tools/clang-tidy-plugin/test-include" )

if config.cata_clang_tidy_executable == "ON":
if config.cata_clang_tidy_executable in ["TRUE", "ON"]:
cata_plugin = ''
else:
cata_plugin = os.path.join(
Expand Down
10 changes: 5 additions & 5 deletions tools/clang-tidy-plugin/test/use-localized-sorting.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ bool operator<( const NonString &, const NonString &rhs ) noexcept;
bool f0( const std::string &l, const std::string &r )
{
return l < r;
// CHECK-MESSAGES: warning: Raw comparison of 'const std::string' (aka 'const basic_string<char>'). For UI purposes please use localized_compare from translations.h. [cata-use-localized-sorting]
// CHECK-MESSAGES: warning: Raw comparison of 'const std::string' (aka '{{.*basic_string.*}}'). For UI purposes please use localized_compare from translations.h. [cata-use-localized-sorting]
}

bool f1( const NonString &l, const NonString &r )
Expand All @@ -25,7 +25,7 @@ bool f1( const NonString &l, const NonString &r )
bool f2( const std::pair<int, std::string> &l, const std::pair<int, std::string> &r )
{
return l < r;
// CHECK-MESSAGES: warning: Raw comparison of 'const std::pair<int, std::string>' (aka 'const pair<int, basic_string<char>>'). For UI purposes please use localized_compare from translations.h. [cata-use-localized-sorting]
// CHECK-MESSAGES: warning: Raw comparison of 'const std::pair<int, std::string>' (aka '{{.*basic_string.*}}'). For UI purposes please use localized_compare from translations.h. [cata-use-localized-sorting]
}

bool f3( const std::pair<int, NonString> &l, const std::pair<int, NonString> &r )
Expand All @@ -36,7 +36,7 @@ bool f3( const std::pair<int, NonString> &l, const std::pair<int, NonString> &r
bool f4( const std::tuple<int, std::string> &l, const std::tuple<int, std::string> &r )
{
return l < r;
// CHECK-MESSAGES: warning: Raw comparison of 'const std::tuple<int, std::string>' (aka 'const tuple<int, basic_string<char>>'). For UI purposes please use localized_compare from translations.h. [cata-use-localized-sorting]
// CHECK-MESSAGES: warning: Raw comparison of 'const std::tuple<int, std::string>' (aka '{{.*basic_string.*}}'). For UI purposes please use localized_compare from translations.h. [cata-use-localized-sorting]
}

bool f5( const std::tuple<int, NonString> &l, const std::tuple<int, NonString> &r )
Expand All @@ -48,13 +48,13 @@ bool f4( const std::tuple<std::tuple<std::string>> &l,
const std::tuple<std::tuple<std::string>> &r )
{
return l < r;
// CHECK-MESSAGES: warning: Raw comparison of 'const std::tuple<std::tuple<std::string>>' (aka 'const tuple<tuple<basic_string<char>>>'). For UI purposes please use localized_compare from translations.h. [cata-use-localized-sorting]
// CHECK-MESSAGES: warning: Raw comparison of 'const std::tuple<std::tuple<std::string>>' (aka '{{.*basic_string.*}}'). For UI purposes please use localized_compare from translations.h. [cata-use-localized-sorting]
}

void sort0( std::string *start, std::string *end )
{
std::sort( start, end );
// CHECK-MESSAGES: warning: Raw sort of 'std::string' (aka 'basic_string<char>'). For UI purposes please use localized_compare from translations.h. [cata-use-localized-sorting]
// CHECK-MESSAGES: warning: Raw sort of 'std::string' (aka '{{.*basic_string.*}}'). For UI purposes please use localized_compare from translations.h. [cata-use-localized-sorting]
}

void sort1( NonString *start, NonString *end )
Expand Down
2 changes: 1 addition & 1 deletion tools/clang-tidy-plugin/test/use-string_view.cpp
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// RUN: %check_clang_tidy -allow-stdinc %s cata-use-string_view %t -- -load=%cata_plugin -- -isystem %cata_include
// RUN: %check_clang_tidy -allow-stdinc %s cata-use-string_view %t -- -load=%cata_plugin -- -isystem %cata_include -fno-delayed-template-parsing

#include <string>
#include <string_view>
Expand Down

0 comments on commit e31b049

Please sign in to comment.