-
Notifications
You must be signed in to change notification settings - Fork 167
Read extended module from SymbolGraph. #1397
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
base: main
Are you sure you want to change the base?
Conversation
…le's name. This is a companion change to the change in the Swift compiler to emit the "extended" module as an additional field in the symbol graph. Reading the base and extended modules from the file name is not feasible for module's with long names. A new compiler option was added in [Swift #83782](swiftlang/swift#83782) to allow emitting symbol graph file's without both module names in the file name. The shortened format isn't yet supported by swift-docc though. This also relies on a corresponding change to swift-docc-symbolkit where `GraphCollector.moduleNameFor` is updated. The change here consolidates some `moduleNameFor` functionality where swift-docc was implementing similar behavior as exists in swift-docc-symbolkit, while already depending on symbolkit so it seems prudent to just share the implementation of `moduleNameFor`.
d-ronnqvist
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I understand correctly, this requires new symbol graph information (the changes to the Swift compiler and SymbolKit) to identify the extended module (because the current code to do is being deleted).
If that's correct, my understanding is that DocC wouldn't be able to correctly identify the extended module for a symbol graph created by any current version of the Swift compiler, or any symbol graphs from Clang or any other source of symbol graph files.
Completely breaking backwards compatibility and breaking compatibility with other sources of symbol graph files would be a deal breaker for this (assuming that I've understood the consequences correctly).
I agree that backwards compatibility important, and breaking swift-docc for the current compiler version would be untenable. But I don't think this change, nor any of the companion changes, breaks backwards compatibility. The change to swift-docc is aimed at reducing duplication, since both swift-docc and swift-docc-symbolkit have a version of For the companion changes, in swift-docc-symbolkit, I'm proposing a change that is also backwards compatible. When the As far as I can tell, each of these changes in isolation and all changes together maintain backwards compatibility. Could you elaborate any on why you believe this is not backwards compatible? Thanks! |
It is true that this PR is deleting code for |
Summary
This is a companion change to the change in the Swift compiler to emit the "extended" module as an additional field in the symbol graph. Reading the base and extended modules from the file name is not feasible for module's with long names. A new compiler option was added in Swift #83782 to allow emitting symbol graph file's without both module names in the file name. The shortened format isn't yet supported by swift-docc though.
This also relies on a corresponding change to swift-docc-symbolkit where
GraphCollector.moduleNameForis updated.The change here consolidates some
moduleNameForfunctionality where swift-docc was implementing similar behavior as exists in swift-docc-symbolkit, while already depending on symbolkit so it seems prudent to just share the implementation ofmoduleNameFor.#1399
Backwards Compatibility
swift-docc must continue to function with symbolgraph data generated by previous versions of the swift toolchain (before swiftlang/swift#86057). This change to swift-docc delegates computing the module name for grouping symbols from swift-docc to existing functionality inside swift-docc-symbolkit, which is already a dependency of the former. The
moduleNameForimplementation in swift-docc-symbolkit today detects the extended module name by extracting it from aModuleNameA@ModuleNameBstyle file name.In the related change, to swift-docc-symbolkit, the functionality that is being used from swift-docc-symbolkit is also updated in a backwards compatible way.
GraphCollector.moduleNameForcontinues to use the existing implementation if the newly addedextendedfield is not present in the symbolgraph.Dependencies
Testing
Describe how a reviewer can test the functionality of your PR. Provide test content to test with if
applicable.
Steps:
Checklist
Make sure you check off the following items. If they cannot be completed, provide a reason.
./bin/testscript and it succeededNo new tests added because all behavior is expected to be identical and existing tests are passing.