Skip to content

[ParseableInterfaces] Short-circuit module loading in the PIML #23857

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 2 commits into from
Apr 8, 2019

Conversation

harlanhaskins
Copy link
Contributor

@harlanhaskins harlanhaskins commented Apr 8, 2019

Previously, the ParseableInterfaceModuleLoader relied on the assumption
that, if it returned errc::not_supported, it would fall through the
search paths and then move on to the SerializedModuleLoader. This did
not anticipate the possibility of a valid .swiftinterface coming later
in the search paths, which can cause issues for the standard library
which is in the resource-dir and should always be loaded from there.

Instead, make the module loading explicitly short-circuit when seeing
errc::not_supported, and document it.

Also add some more logging throughout discoverLoadableModule so we can
more easily catch issues like this in the future.

Fixes rdar://49479386

@harlanhaskins
Copy link
Contributor Author

harlanhaskins commented Apr 8, 2019

This seems a better approach than loading directly from the PIML -- it makes explicit an assumption from before, and it does properly handle the existence of a module without an interface. Seeing that, it will fall back to the serialized loader and skip the parseable interface loader.

Harlan Haskins added 2 commits April 8, 2019 10:07
Previously, the ParseableInterfaceModuleLoader relied on the assumption
that, if it returned `errc::not_supported`, it would fall through the
search paths and then move on to the SerializedModuleLoader. This did
not anticipate the possibility of a valid .swiftinterface coming later
in the search paths, which can cause issues for the standard library
which is in the resource-dir and should always be loaded from there.

Instead, make the module loading explicitly short-circuit when seeing
`errc::not_supported`, and document it.

Also add some more logging throughout `discoverLoadableModule` so we can
more easily catch issues like this in the future.

Fixes rdar://49479386
@jrose-apple
Copy link
Contributor

Minor concern about not_supported being generated from a filesystem operation, but I guess that seems pretty unlikely for the "read"- and "stat"-like operations we're doing here. LGTM, then.

@harlanhaskins
Copy link
Contributor Author

@swift-ci please test

@swift-ci
Copy link
Contributor

swift-ci commented Apr 8, 2019

Build failed
Swift Test Linux Platform
Git Sha - c2da74ab734f3ca535cdd1b870b5c37cb7b52efd

@swift-ci
Copy link
Contributor

swift-ci commented Apr 8, 2019

Build failed
Swift Test OS X Platform
Git Sha - c2da74ab734f3ca535cdd1b870b5c37cb7b52efd

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