-
Notifications
You must be signed in to change notification settings - Fork 769
Provide a way to create a compilation DB in the bazel project #9122
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
Provide a way to create a compilation DB in the bazel project #9122
Conversation
|
clang-tidy review says "All clean, LGTM! 👍" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request introduces useful scripts for generating a compilation database and running clang-tidy, which is a great improvement for developer workflow and code quality. The scripts are well-structured. My review includes several suggestions to enhance their robustness and maintainability by following shell scripting best practices. These include adding error handling (set -e), quoting variables and command substitutions to prevent word splitting issues, and using more robust methods like bazel cquery to locate build artifacts instead of relying on fragile path globbing. I've also pointed out a potential bug in a loop that could occur if a glob pattern doesn't match any files.
ead2044 to
930e60b
Compare
|
clang-tidy review says "All clean, LGTM! 👍" |
930e60b to
5eb9f4a
Compare
|
clang-tidy review says "All clean, LGTM! 👍" |
5eb9f4a to
3a228ad
Compare
|
clang-tidy review says "All clean, LGTM! 👍" |
This uses http://bant.build/ to create a compilation DB. The change provides two useful scripts: * `etc/bazel-make-compilation-db.sh` creates a compilation db. To avoid a huge compile_commands.json, this generates the compile_flags.txt which is a much shorter file applied to all files. This keeps memory usage low for tools such as clang-tidy * `etc/run-clang-tidy.sh` a convenience script that creates the compilation DB and then calls `run-clang-tidy-cached.cc`. It uses `clang-tidy` from the llvm-toolchain configured in the MODULE.bazel Issues The-OpenROAD-Project#8586 Signed-off-by: Henner Zeller <[email protected]>
3a228ad to
1a2d93e
Compare
|
clang-tidy review says "All clean, LGTM! 👍" |
|
(forgot to make it a separate commit, so it is |
|
compile_flags.txt seems to try to be a universal set of includes rather than the specific includes used by a given translation unit. I don't see how that will work in general. For example: build/compile_commands.json is 3.4M so larger but not terrible. cmake is much faster at this job as it doesn't require a build. I think you only need to generate the swig wrappers and tcl encoding files. |
Signed-off-by: Henner Zeller <[email protected]>
|
clang-tidy review says "All clean, LGTM! 👍" |
1 similar comment
|
clang-tidy review says "All clean, LGTM! 👍" |
Yeah, that is a general problem of the project layout which I have said many times before. Once we can abandon CMake, we should fix the paths to always use paths relative to project root.
For bazel, it would be several hundred megabytes, as it needs to encode all the paths. With that, running clang-tidy on 128 cores will require several hundred gigabytes of RAM. So unfortunately, a compile_commands.json is not an option right now.
We do that now also but build the relevant generated files. Ideally,
Yep, changed that now, so only the fetch of the dependencies is done plus the handful of generated things (much faster) |
Signed-off-by: Henner Zeller <[email protected]>
080d3a9 to
2ad0129
Compare
|
clang-tidy review says "All clean, LGTM! 👍" |
|
Thank you for your work Henner, just confirming this works exactly as expected from my side, including from clangd. The one observation I do have is that clangd does not seem to generate its symbol database without a compile_commands.json, meaning that it has to re-index the project on the fly as you edit; which is a bit slow in some large files, particularly OpenDB-related things. I expect that clangd resource usage will be high, but more tame than clang-tidy. If Bant supports generating compile_commands without too much extra trouble, it would be interesting to try and see how this behaves. |
|
The above ended up being simple enough to do, so I've got some numbers on it. The Machine specs are 32 cores, 64GB DDR4 RAM. |
|
There are a lot of paths that are needed for bazel, as each external dependency has its own lengthy path. The paths bant emits are also for possible generated sources, so there might be a superset that is not needed in practice. Anyway, ready to merge ? |
|
Thanks we can always refine more later. |
This uses http://bant.build/ to create a compilation DB.
The change provides two useful scripts:
etc/bazel-make-compilation-db.shcreates a compilation db. To avoid a huge compile_commands.json, this generates the compile_flags.txt which is a much shorter file applied to all files. This keeps memory usage low for tools such as clang-tidyetc/run-clang-tidy.sha convenience script that creates the compilation DB and then callsrun-clang-tidy-cached.cc. It usesclang-tidyfrom the llvm-toolchain configured in the MODULE.bazelIssues #8586