Enable automatic generation of compiler flag list for Clang tooling#2008
Enable automatic generation of compiler flag list for Clang tooling#2008ISSOtm wants to merge 1 commit into
Conversation
ISSOtm
added
enhancement
Rangi42
left a comment
Rangi42
left a comment
There was a problem hiding this comment.
Looks pretty good! A few suggestions and questions.
| *.gcno | ||
| *.gcda | ||
| *.gcov | ||
| /compile_flags.txt |
There was a problem hiding this comment.
Why a slash on this one and not on the other root-only ignored files?
There was a problem hiding this comment.
Because this file should only be present at the root, whereas the other files are (as far as I'm aware) possible in all subdirs?
There was a problem hiding this comment.
Others like CMakeUserPresets.json, cmake_install.cmake, *.dSYM/, etc should only end up in the root too. Sure we probably don't want to commit them if they somehow get created elsewhere, but the same is true for compile_flags.txt.
There was a problem hiding this comment.
Well, the CMake-related files are because one can make build dirs in places other than build/, so it was a sort of defensive thing.
The other files (*.dSYM/, *.gcov...) I have no idea where they're supposed to end up, so I have not attempted to correct them. If they are only supposed to be present in the root, I'd rather have their patterns anchored like this one.
| tidy: src/asm/parser.hpp src/link/script.hpp | ||
| tidy: src/asm/parser.hpp src/link/script.hpp compile_flags.txt | ||
| $Qclang-tidy -p . $$(git ls-files '*.[hc]pp') | ||
| # Note that this file has priority over `compile_commands.json`, but is less precise. |
There was a problem hiding this comment.
| # Note that this file has priority over `compile_commands.json`, but is less precise. | |
| # Note that `compile_flags.txt` has priority over `compile_commands.json`, but is less precise. |
There was a problem hiding this comment.
Also, what does "less precise" mean?
There was a problem hiding this comment.
The JSON file is able to specify different options for each file (e.g. RGBGFX getting the libpng flags), but the .txt is, by design, not.
| # The JSON file can be generated by CMake (`CXX=clang++ cmake -B build -DCMAKE_EXPORT_COMPILE_COMMANDS`), | ||
| # building (`cmake --build build`), and then invoking `clang-tidy -p build <files>`. |
There was a problem hiding this comment.
"The JSON file" being compile_commands.json, under what circumstances would a user need or want to generate one? If they might need to, why isn't there a compile_commands.json: target? (And if they don't ever need to, IMO let's not mention it.)
There was a problem hiding this comment.
This file would be Very Fucking Annoying to generate via Make. CMake can generate it, but that should be done as part of the build itself, and besides that requires a different invocation of clang-tidy.
2 participants
Fixes #1995, removing one source of truth for our flags.
I have considered removing
REAL{CXX,LD}FLAGSand replacing them with the three variables, but idk if that's desirable.