Skip to content

Commit 80ac7ef

Browse files
committed
Fix minor typos
1 parent 1df668a commit 80ac7ef

File tree

1 file changed

+3
-3
lines changed

1 file changed

+3
-3
lines changed

ImplementationReadme.md

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ The `refresh_compile_commands` rule (from [`refresh_compile_commands.bzl`](./ref
6262
Why?
6363

6464
- LLVM projects are well-built, best-poised to understand languages, and well-backed.
65-
- You want tooling that understands the language. Compiler frontends understand languages best. Tooling that builds on the best compiler frontend—and is written by the same folks—is likely to be best.
65+
- You want tooling that understands the language. Compiler front ends understand languages best. Tooling that builds on the best compiler frontend—and is written by the same folks—is likely to be best.
6666
- The community is great: `clangd` is very responsive on GitHub, it's integrated in many editors (including VSCode), and there are many deep pockets that need it and fund it.
6767
- The alternatives have key drawbacks:
6868
- The Microsoft VSCode plugin for C++ doesn't support Objective-C (which we need), would more tightly couple us to VSCode with a non-standard compile commands format, doesn't seem to be based on a compiler, and has had various issues parsing platform command flags.
@@ -140,11 +140,11 @@ We're using bazel's [`aquery` ("action query")](https://docs.bazel.build/version
140140

141141
Previously, we'd built things using [`action_listeners`](https://docs.bazel.build/versions/master/be/extra-actions.html). (The old, original implementation lives in our git history). Action listeners are good in that, like `aquery`, they directly listen at the level of compile actions. However, they can only be invoked at the same time as a full build. This makes the first generation *really* slow—like 30m instead of aquery's 30s. Rebuilds that should hit a warm cache are also slow (~10m) if you have multiple targets, which has to be an [unnecessary cache miss bug in Bazel](https://github.com/bazelbuild/bazel/issues/13029). You might be able to get around the 30m cold issue by suppressing build with [`--nobuild`](https://docs.bazel.build/versions/master/command-line-reference.html#flag--build), though we haven't tried it and it might disrupt the output protos or otherwise not work. (Only haven't tried because we switched implementations before finding the flag.) Another key downside compared to `aquery` is that `action_listener` output accumulates in the build tree, and there's no no way to tell which outputs are fresh. There's always the risk that stale output could bork your `compile_commands.json`, so you need to widen the user interface to include a cleaning step. An additional issue is future support: action listeners are marked as deprecated in their docs, even if they are still used (at the time of writing) by Google's Kythe tooling. The main (but hopeful temporary) benefit of action listeners is that they auto-identify headers used by a given source file, which makes working around the [header commands inference issue](https://github.com/clangd/clangd/issues/123) easier. Another smaller benefit is that being integrated with building guarantees generated files have been generated. So while `action_listeners` can get the job done, they're slow and the abstraction is leaky and questionably supported.
142142

143-
What you really don't want to do is use `bazel query`, `bazel cquery`, or `bazel aspect` if you want compile commands. The primary problem with each is that they crawl the graph of bazel targets (think graph nodes), rather than actually listening to the commands Bazel is going to invoke during its compile actions. We're trying to output compile commands, not graph nodes, and it's not a 1:1 relationship! Rather than getting the commands directly with aquery, you'd have to duplicate the rule attribute -> command line logic from bazel & the toolchain, which is tough with custom toolchains, platform-specific logic, and in-graph configuration transitions for cross-compiling. Note that `query` has issues with `select()` statements, unlike `cquery`, which takes configuration into account (hence "configured query"->cquery). Aspects are also problematic because they only propagate along attributes (like `"deps"`), breaking the chain at things like genrules, which name things differently (`"srcs"` not `"deps"` for genrules) vs. easily sorting to the right mnemonic compile actions with aquery even for actions that are for code-gen tools being built for the host. But most importantly, `query`, `cquery`, and `aspect` share the key fundamental flaw of operating on graph nodes not action-graph edges.
143+
What you really don't want to do is use `bazel query`, `bazel cquery`, or `aspect`s if you want compile commands. The primary problem with each is that they crawl the graph of bazel targets (think graph nodes), rather than actually listening to the commands Bazel is going to invoke during its compile actions. We're trying to output compile commands, not graph nodes, and it's not a 1:1 relationship! Rather than getting the commands directly with aquery, you'd have to duplicate the rule attribute -> command line logic from bazel & the toolchain, which is tough with custom toolchains, platform-specific logic, and in-graph configuration transitions for cross-compiling. Note that `query` has issues with `select()` statements, unlike `cquery`, which takes configuration into account (hence "configured query"->cquery). Aspects are also problematic because they only propagate along attributes (like `"deps"`), breaking the chain at things like genrules, which name things differently (`"srcs"` not `"deps"` for genrules) vs. easily sorting to the right mnemonic compile actions with aquery even for actions that are for code-gen tools being built for the host. But most importantly, `query`, `cquery`, and `aspect` share the key fundamental flaw of operating on graph nodes not action-graph edges.
144144

145145
### Caveats
146146

147-
As far as we know, the only (unsolvable) problem with aquery is that it can't get the command line when sources are undetermined, like a tree artifact. Thankfully, this seems to be a very rare case edge case, handled by Bazel...but only hackily and requiring a directory with a source extension. I'm imagining that it's mostly for generated sources--which you don't much care about editing--since manually-written files can be statically listed. (Note that there isn't such a problem with headers, since we'll find them when we traverse the include graph.) See [this discussion](https://github.com/grailbio/bazel-compilation-database/pull/89#discussion_r1140927118) for an example of the problem case and some more details. If you experiment with the example therein, you'll find the aquery results for the tree artifact cc_library contain an action with the menmonic "CppCompileActionTemplate" instead of "CppCompile" (so we'll automatically ignore these tree actions) and that there's no command line or arguments for that CppCompileActionTemplate action.
147+
As far as we know, the only (unsolvable) problem with aquery is that it can't get the command line when sources are undetermined, like a tree artifact. Thankfully, this seems to be a very rare case edge case, handled by Bazel...but only hackily and requiring a directory with a source extension. I'm imagining that it's mostly for generated sources--which you don't much care about editing--since manually-written files can be statically listed. (Note that there isn't such a problem with headers, since we'll find them when we traverse the include graph.) See [this discussion](https://github.com/grailbio/bazel-compilation-database/pull/89#discussion_r1140927118) for an example of the problem case and some more details. If you experiment with the example therein, you'll find the aquery results for the tree artifact cc_library contain an action with the mnemonic "CppCompileActionTemplate" instead of "CppCompile" (so we'll automatically ignore these tree actions) and that there's no command line or arguments for that CppCompileActionTemplate action.
148148

149149
Aspects may yet be useful for generating header files without a complete build, although building the subset you care about is a common enough part of development that this hasn't been a big issue in practice. See [this issue](https://github.com/bazelbuild/bazel/issues/17660) we filed for more details on approaches, a discussion of future Bazel support, and a link to rules_xcodeproj's implementation.
150150

0 commit comments

Comments
 (0)