Skip to content

Render symbol graphs for cross-import overlay modules, if present #34922

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 5 commits into from
Dec 12, 2020

Conversation

QuietMisdreavus
Copy link
Contributor

@QuietMisdreavus QuietMisdreavus commented Dec 2, 2020

Currently, the symbol graph tool does not emit information about cross-import overlay modules that the given module declares. This PR proposes probing the currently-known module list after rendering the main symbol graph to determine whether there are any underscore-named modules which overlay the given one, and then rendering additional symbol graphs for them as well, as if they were extensions.

This change includes a new field in the symbol graph format, module.bystanders, which lists the "bystander modules" that must be imported alongside the main module.name module to access the overlay's items. The presence of module.bystanders applies to all the symbols in the file. Because of the new field, i bumped the symbol graph file format version to 0.5.1.

Resolves rdar://59688006

@QuietMisdreavus
Copy link
Contributor Author

@swift-ci Please smoke test

@akyrtzi
Copy link
Contributor

akyrtzi commented Dec 9, 2020

@brentdax could you review since this is about cross-import overlays?

Copy link
Contributor

@beccadax beccadax left a comment

Choose a reason for hiding this comment

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

Looks pretty good. I'm going to mark this Approved because I'm not sure that you need to do anything about the filename issue, but please read my comment there before you merge this.

@QuietMisdreavus QuietMisdreavus force-pushed the QuietMisdreavus/overlay-symbol-graph branch from 36d0528 to bf9274d Compare December 11, 2020 19:23
@QuietMisdreavus
Copy link
Contributor Author

@swift-ci Please smoke test

Copy link
Contributor

@beccadax beccadax left a comment

Choose a reason for hiding this comment

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

Looks good to me!

@QuietMisdreavus
Copy link
Contributor Author

The cmark tests seem to have failed in my CI run? I don't think i touched anything related to that here.

@QuietMisdreavus
Copy link
Contributor Author

I wonder if this is related to this bug: https://bugs.swift.org/browse/SR-13635

@QuietMisdreavus
Copy link
Contributor Author

Going to try again and blame some flaky cmark tests...

@swift-ci Please clean smoke test and merge

@swift-ci swift-ci merged commit 72fc06c into main Dec 12, 2020
@swift-ci swift-ci deleted the QuietMisdreavus/overlay-symbol-graph branch December 12, 2020 04:02
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.

4 participants