Skip to content

add default values for SymbolGraphOptions #59037

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

Merged
merged 4 commits into from
May 26, 2022

Conversation

QuietMisdreavus
Copy link
Contributor

@QuietMisdreavus QuietMisdreavus commented May 23, 2022

Resolves rdar://93780666

Currently, the IncludeClangDocs field of SymbolGraphOptions is uninitialized when generating a symbol graph during a build. The assumption behind this field is that the branches that it gates would only be run when generating a symbol graph from SourceKit. However, we've noticed a situation where the symbol graph is emitting the partial "location" field during a build on ARM macOS. This PR adds defaults to the SymbolGraphOptions struct, as well as manually setting the IncludeClangDocs field to false when building, to ensure that this code path is never hit during a regular compile.

@QuietMisdreavus
Copy link
Contributor Author

@swift-ci Please build toolchain macOS

@QuietMisdreavus QuietMisdreavus force-pushed the QuietMisdreavus/clang-syms-while-building branch from f682cc6 to b42b0ed Compare May 25, 2022 15:17
@QuietMisdreavus QuietMisdreavus marked this pull request as ready for review May 25, 2022 15:21
@QuietMisdreavus
Copy link
Contributor Author

@swift-ci Please test

@QuietMisdreavus QuietMisdreavus changed the title manually set SymbolGraphOptions.IncludeClangDocs to false add default values for SymbolGraphOptions May 25, 2022
Comment on lines 24 to 27
StringRef OutputDir = StringRef{};

/// The target of the module.
llvm::Triple Target;
llvm::Triple Target = llvm::Triple{};
Copy link
Contributor

Choose a reason for hiding this comment

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

These are default initialized already, but providing it doesn't hurt. You could just do = {}; if you want to keep though.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm being extra paranoid now, since i was assuming that bool had a default initialization as well. 😅 I can clean up the initializer value, though. Thanks for the tip!

@QuietMisdreavus
Copy link
Contributor Author

@swift-ci Please test

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