Skip to content
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

Generate compile_commands.json compilation database #944

Closed

Conversation

matthijskooijman
Copy link
Collaborator

@matthijskooijman matthijskooijman commented Sep 11, 2020

Please check if the PR fulfills these requirements

  • The PR has no duplicates (please search among the Pull Requests
    before creating one)
  • The PR follows
    our contributing guidelines
  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)
  • What kind of change does this PR introduce?
    New feature: Allow generating compile_commands.json file as a side effect of compilation, for consumption by editors and other tooling, as requested in Feature Request: Export compile_commands.json #849.

  • What is the new behavior?
    All compile commands (not linking and other steps) ran are output to a compilation database, a file called compile_commands.json (see https://clang.llvm.org/docs/JSONCompilationDatabase.html for the format). This file can be used by editors (often using clang or clangd) to provide code completion, navigation and diagnostics.

  • Other information:
    This is just a rough draft, that still needs more work, some decisions and some feedback. I wanted to quickly throw something together, which ended up a bit more work than I thought. The basics work, though this is probably more of a proof of concept that might need to be significantly restructured. Open points are:

  • How to log: introduced a CompilationDatabase class outside of the legacy/builder directory (seemed good to not add more legacy), but now I'm unsure how to approach logging. It seems that the cli and builder code have distinct logging facilities, neither of which seems easily accessible. For now I just added "fmt.Print". Suggestions are welcome.
  • Where to write the file: I now write a compile_commands.json to the sketch directory, since that's where editors would look for it. I later realized that it would probably be good to also write a version to the temp build directory (note: I mean the /tmp/arduino-sketch-XYZ directory, not the build subdirectory of the sketch dir where the compilation result is exported), for any tooling that wants to inspect the copied and preprocessed sketch files.
  • Updating sketch paths: Currently, the paths are written as they passed to the compiler, so pointing at the copy of the sketch in the temp build dir. This is probably good for the compile_commands.json to be written to the temp build dir, but the sketch dir version should really point to the original sketch files. This means that the file element in the json should be updated, but maybe also any compiler arguments that resolve to sketch files should have their paths updated. This should probably be applied generically (i.e. search replacing back to the original sketch dir) to all arguments, since we can't be sure what a platform.txt does exactly. Alternatively, updating just the file field might be sufficient and can be done by passing the original filename around (does require significant refactoring, I think).
  • Handling .ino files: Currently, only one entry is generated for .cpp file resulting from the combined and preprocessed .ino file(s). This is again fine for the temp build version, but for the sketch dir version, I think this should have a separate entry for each .ino file, with an -include=Arduino.h flag added (as suggested in Feature Request: Export compile_commands.json #849 (comment)). This is imperfect, but probably as close as we can get.
  • Making this optional: I believe that writing a compile_commands.json to the temp build dir can always be done, but writing to the sketch directory should almost certainly be opt-in (through commandline or config), to prevent user surprise and because the IDE and arduino-cli seem to try hard to prevent touching the sketch dir now. For arduino-cli, it might make sense to always generate the file in the build subdirectory, along with the compiled program, and provide an option to also, or instead of, generate in the sketch directory itself (where it will be mostly automatically picked up by editors or clangd). For the IDE, some extra care should probably be taken to not generate this file in the sketch directory for unmodified examples, even if the user enabled it in the config.
  • Partial compilation: Currently, the file is written from scratch at the end of every compilation. So if compilation fails, or files are not changed and thus not recompiled, files will be missing from the compile_commands.json file. Ideally, any existing file should be read and merged, taking care to remove any extra entries (e.g. for deleted files), but only when compilation was successful (or at least complete). Alternatively, a complete list of files to be compiled could also be used maybe (even when compilation is aborted halfway).

This prepares for building a compilation database later. The returned
command is not currently used anywhere yet, so this commit should not
change behaviour.
This is still very rough and unfinished.
@cmaglie
Copy link
Member

cmaglie commented Sep 15, 2020

Handling .ino files: Currently, only one entry is generated for .cpp file resulting from the combined and preprocessed .ino file(s). This is again fine for the temp build version, but for the sketch dir version, I think this should have a separate entry for each .ino file, with an -include=Arduino.h flag added (as suggested in #849 (comment)).

does the added #line directive as suggested in #707 (as we do in the preprocessed *.ino -> .cpp) may solve this?

@matthijskooijman
Copy link
Collaborator Author

does the added #line directive as suggested in #707 (as we do in the preprocessed *.ino -> .cpp) may solve this?

I think it does not, since I think editors will just request an analysis of the file that is being edited, which does not have these directives. For those line directives to be helpful, a language server such as clangd would have to analyze all files in the compilation database (which it already does, I think), then see the #line directives and then somehow conclude that the flags of the preprocessed copy apply to the .ino files (which I don't think it does, or could reliably do, since #line directives can very well be the result of an #include too). Even more, to really fix the issues coming from preprocessing, clangd should be told to analyze the preprocessed copy (which I think requires changes in the editor and cannot be done through the compilation database), and then generate diagnostics for the original (rather than the actually being analyzed) file based on the #line directives (just tested this quickly, this is not what clangd does now).

So I'm afraid that this is just the hole we dug with this preprocessing. Anyone who is serious about using the compilation database with a third-party editor should probably use the "empty .ino file"-approach to work around this.

@vivi90
Copy link

vivi90 commented Oct 29, 2020

I need this compile_commands.json file for the code check and completion by the ide-cpp plugin of my Atom editor.
Without it i get the message Unable to handle compilation, expected exactly one compiler job in '' by clangd.
See also: thomasjo/atom-ide-cpp#19

@matthijskooijman
Copy link
Collaborator Author

This is superseded by the already merged #1081.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants