Open Bug 1583999 Opened 2 months ago Updated 5 days ago

Make the CompileDB backend work with clangd

Categories

(Firefox Build System :: Developer Environment Integration, enhancement)

enhancement
Not set

Tracking

(Not tracked)

People

(Reporter: botond, Unassigned)

Details

Attachments

(1 file)

clangd is a C++ language server that provides a lot of useful semantics-aware editing features for any editor that supports the Language Server Protocol.

Getting clangd to understand a codebase requires a compile_commands.json file. We have machinery to generate one, mach build-backend -b CompileDB.

Unfortunately, clangd doesn't work well with our compile_commands.json file due to unified compilation.

The entries in our compile_commands.json file look something like this:

{
"directory": "<objdir>/gfx/layers",
"command": "clang++ <flags> <objdir>/gfx/layers/Unified_gfx_layers1.cpp",
"file": "<srcdir>/gfx/layers/AnimationHelper.cpp"
},

Note that the value of the "file" key, which is the non-unified source file, does not match the actual input file to the compile command, which is the unified source file.

This confuses clangd, which expects the two to be the same. (Specifically, clangd will interpret the contents of the non-unified file as if it were located in the directory where the unified file is, which breaks resolution of relative include paths.)

We should fix our CompileDB backend to generate a compile_commands.json that clangd can consume.

Either of the following would work:

  • Disable unified compilation for the purposes of generating the CompileDB
  • Emit the unified source file under the "file" key as well, thereby having it match the actual source file used in the command.

I'd argue it's clangd that needs fixing.

(In reply to Mike Hommey [:glandium] from comment #1)

I'd argue it's clangd that needs fixing.

I would find it hard to defend our current format. The spec for compilation databases says:

The contracts for each field in the command object are:

  • directory: The working directory of the compilation. All paths specified in the command or file fields must be either absolute or relative to this directory.
  • file: The main translation unit source processed by this compilation step. This is used by tools as the key into the compilation database. There can be multiple command objects for the same file, for example if the same source file is compiled with different configurations.
  • command: The compile command executed. After JSON unescaping, this must be a valid command to rerun the exact compilation step for the translation unit in the environment the build system uses. Parameters use shell quoting and shell escaping of quotes, with ‘"’ and ‘\’ being the only special characters. Shell expansion is not supported.

Note in particular that file is "The main translation unit source processed by this compilation step." If we're doing a unified build, the non-unified file is not the "main translation unit source", it's just an included file.

(In reply to Botond Ballo [:botond] from comment #2)

(In reply to Mike Hommey [:glandium] from comment #1)

I'd argue it's clangd that needs fixing.

I would find it hard to defend our current format.

I tend to agree. Isn't the point of compile_commands.json to be consumed by tooling? We should do whatever the tooling needs (perhaps while pursuing improvements in parallel).

On the subject of the spec, it rather looks like unrecognized fields aren't allowed. If that's not correct, could we include a special, Mozilla-only, "non-unified" field that would capture the current "file" field so we don't lose that information?

Just FYI, since I happen to be looking at compile_commands.json for some sccache stuff right now: my DB has an entry with file: /Users/nalexander/Mozilla/objdirs/objdir-droid-compile/gfx/layers/Unified_cpp_gfx_layers0.cpp as well as ones for subpieces of that unified source. Is that expected?

FWIW, the attached simple change makes clangd work for me locally.

I'm not sure if this change is landable as written.

You need to log in before you can comment on or make changes to this bug.