Skip to content

Add feature to produce serialized diagnostics files#15403

Closed
brentleyjones wants to merge 1 commit intobazelbuild:masterfrom
brentleyjones:bj/add-feature-to-produce-serialized-diagnostics-files
Closed

Add feature to produce serialized diagnostics files#15403
brentleyjones wants to merge 1 commit intobazelbuild:masterfrom
brentleyjones:bj/add-feature-to-produce-serialized-diagnostics-files

Conversation

@brentleyjones
Copy link
Copy Markdown
Contributor

@brentleyjones brentleyjones commented May 4, 2022

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 brentleyjones requested a review from lberki as a code owner May 4, 2022 15:28
@brentleyjones brentleyjones mentioned this pull request May 4, 2022
Closed
@brentleyjones
Copy link
Copy Markdown
Contributor Author

brentleyjones commented May 4, 2022

cc @oquenchil @meteorcloudy @DavidGoldman

@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
Copy Markdown
Member

meteorcloudy commented May 13, 2022

@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
Copy Markdown
Contributor

oquenchil commented May 18, 2022

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
Copy Markdown
Contributor Author

brentleyjones commented May 18, 2022

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

@brentleyjones
Copy link
Copy Markdown
Contributor Author

brentleyjones commented May 18, 2022

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
@brentleyjones
Add feature to produce serialized diagnostics files
125f563 
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
Copy Markdown
Contributor Author

brentleyjones commented May 23, 2022

Friendly ping @oquenchil.

oquenchil
oquenchil approved these changes May 25, 2022
View reviewed changes
Copy link
Copy Markdown
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
Copy Markdown
Contributor Author

brentleyjones commented May 25, 2022

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
@BalestraPatrick BalestraPatrick mentioned this pull request Apr 5, 2023
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@oquenchil oquenchil oquenchil approved these changes

@googlewalt googlewalt Awaiting requested review from googlewalt

Assignees

@googlewalt googlewalt

Labels

team-Rules-CPP Issues for C++ rules

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

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

5 participants

@brentleyjones @meteorcloudy @oquenchil @googlewalt @ckolli5