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

Add feature to produce serialized diagnostics files #15403

Conversation

brentleyjones
Copy link
Contributor

Using the serialized_diagnostics_file feature will add the --serialized-diagnostics flag to C/C++/Objective-C/Objective-C++ compiles, causing a declared.dia file output to be produced.

Closes #15191.

@brentleyjones
Copy link
Contributor Author

@brentleyjones brentleyjones force-pushed the bj/add-feature-to-produce-serialized-diagnostics-files branch from 3fb225c to 49683e2 Compare May 4, 2022 15:35
@brentleyjones brentleyjones requested a review from oquenchil as a code owner May 4, 2022 15:35
@ckolli5 ckolli5 added the team-Rules-CPP Issues for C++ rules label May 5, 2022
@oquenchil oquenchil requested review from googlewalt and removed request for lberki and oquenchil May 5, 2022 07:54
@meteorcloudy
Copy link
Member

@googlewalt Do you think you are the right person to review this?

@oquenchil Since this feature seems not only apply to objc rules, maybe you should also take a look?

@oquenchil
Copy link
Contributor

Hi Brentley, through no fault of your own this involves a lot of piping in the codebase just to tell Bazel about a new file being produced during compilation. Can you think of a way this could work using additional outputs?. I would already consider it a win if we don't have to know about that additional extension.

Thanks.

@brentleyjones
Copy link
Contributor Author

Sure, I'll take a look today to see if I can use that.

@brentleyjones
Copy link
Contributor Author

From the looks of it, that will shift a small amount of code from CppCompileActionBuilder.setOutputs() to CppCompileActionBuilder.setAdditionalOutputs().

Because of it being a feature_flag, we still need to know about the extension and whatnot for addStringVariable(). I'm not really sure there is much of a win in refactoring this, and honestly there is a higher chance of me messing things up when I'm pretty sure the change as is works. I think it can get better if we eventually move to the inject like design (#15191 (comment)) but that is out of scope for this change, as I would like to get this into the 5.2 release as well.

@brentleyjones brentleyjones force-pushed the bj/add-feature-to-produce-serialized-diagnostics-files branch from 49683e2 to 8a92ade Compare May 18, 2022 13:49
Using the `serialized_diagnostics_file` feature will add the `--serialized-diagnostics` flag to C/C++/Objective-C/Objective-C++ compiles, causing a declared`.dia` file output to be produced.
@brentleyjones brentleyjones force-pushed the bj/add-feature-to-produce-serialized-diagnostics-files branch from 8a92ade to 125f563 Compare May 18, 2022 18:27
@brentleyjones
Copy link
Contributor Author

Friendly ping @oquenchil.

Copy link
Contributor

@oquenchil oquenchil left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@brentleyjones
Copy link
Contributor Author

Thanks @oquenchil. Can I get this imported?

@brentleyjones brentleyjones deleted the bj/add-feature-to-produce-serialized-diagnostics-files branch May 31, 2022 13:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
team-Rules-CPP Issues for C++ rules
Projects
None yet
Development

Successfully merging this pull request may close these issues.

FR: Add ability to generate clang serialized diagnostics files for CC builds
5 participants