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

FR: Add ability to generate clang serialized diagnostics files for CC builds #15191

Closed
brentleyjones opened this issue Apr 7, 2022 · 6 comments
Assignees
Labels
P2 We'll consider working on this in future. (Assignee optional) team-Rules-CPP Issues for C++ rules type: feature request

Comments

@brentleyjones
Copy link
Contributor

brentleyjones commented Apr 7, 2022

Description of the feature request:

Allow (probably via a feature) the ability for C/C++/Obj-C files to generate clang serialized diagnostics files (--serialized-diagnostics) for each source file. The file should be handled similar to the .d file, and be a declared dependency.

What underlying problem are you trying to solve with this feature?

I have an IDE integration (Xcode) use case where I want --serialized-diagnostics source_file.dia to be applied to each C/C++/Obj-C compilation, as the IDE consumes this file to report warnings, errors, notes, remarks and fix-its. These files need to be fetched from cache as well, so they need to be a declared output.

Which operating system are you running Bazel on?

macOS

What is the output of bazel info release?

release 5.1.0

If bazel info release returns development version or (@non-git), tell us how you built Bazel.

No response

What's the output of git remote get-url origin; git rev-parse master; git rev-parse HEAD ?

No response

Have you found anything relevant by searching the web?

No response

Any other information, logs, or outputs that you want to share?

I'm willing to contribute this change, if it will be accepted.

@sgowroji sgowroji added team-Rules-CPP Issues for C++ rules untriaged labels Apr 8, 2022
@meteorcloudy meteorcloudy added type: feature request P2 We'll consider working on this in future. (Assignee optional) and removed untriaged labels Apr 13, 2022
@meteorcloudy
Copy link
Member

@oquenchil Do you think it's possible to implement this as an aspect or a custom C++ toolchain?
@brentleyjones If we need to change Bazel's C++ rules, we better have a quick design doc for this. Also C++ rules are being starlarkified, maybe it can be implemented in the C++ starlark rule?

@DavidGoldman
Copy link
Contributor

DavidGoldman commented Apr 17, 2022

This seems like a reasonable FR - -serialize-diagnostic-file and --serialize-diagnostics are both available in upstream Clang (it's not specific to Apple's fork). This should be implemented by modifying the CC/ObjC rules, not via an aspect, since the additional file should be produced by the original compilation action (not via an additional action) - the overhead of producing it is minimal compared to the time to load the AST/TU. Controlling this with a feature that's disabled by default seems reasonable.

@meteorcloudy
Copy link
Member

@brentleyjones I guess implementing this as a feature is relatively simple, feel free to send a PR.

@meteorcloudy
Copy link
Member

@comius @oquenchil In generally, I think we should have a way to "inject" features for the default cc toolchain so that users don't have to fully maintain their own cc toolchain, and can just say I want to add this feature to the default cc toolchain.

@oquenchil
Copy link
Contributor

@comius @oquenchil In generally, I think we should have a way to "inject" features for the default cc toolchain so that users don't have to fully maintain their own cc toolchain, and can just say I want to add this feature to the default cc toolchain.

Totally agree with this and this has been in the back of my mind for years but never had time to work on it. A design doc for functionality that allows this injection would be very welcome.

@brentleyjones
Copy link
Contributor Author

Sorry for the delay. I've opened a PR here: #15403. And I have the cherry-pick for 5.2 ready once that lands.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P2 We'll consider working on this in future. (Assignee optional) team-Rules-CPP Issues for C++ rules type: feature request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants