Skip to content

In performTypeChecking, defer verifyAllLoadedModules in WMO mode. #14250

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
Jan 30, 2018
Merged

In performTypeChecking, defer verifyAllLoadedModules in WMO mode. #14250

merged 2 commits into from
Jan 30, 2018

Conversation

atrick
Copy link
Contributor

@atrick atrick commented Jan 29, 2018

Suggested by Jordan Rose.

Defer per-file verification in whole-module mode. Verifying imports between
files could cause the importer to cache declarations without adding them to the
ASTContext. This happens when the importer registers a declaration without a
valid TypeChecker instance, as is the case during verification. A subsequent
file may require that declaration to be fully imported (e.g. to synthesized a
function body), but since it has already been cached, it will never be added to
the ASTContext. The solution is to skip verification and avoid caching
it. Instead, loaded modules are now verified in WMO mode once during
performWholeModuleTypeChecking.

Fixes rdar:36801676 [Edge][swift 4.1] SIL verification failed: external
declarations of SILFunctions with shared visibility is not allowed.

Suggested by Jordan Rose.

Defer per-file verification in whole-module mode. Verifying imports between
files could cause the importer to cache declarations without adding them to the
ASTContext. This happens when the importer registers a declaration without a
valid TypeChecker instance, as is the case during verification. A subsequent
file may require that declaration to be fully imported (e.g. to synthesized a
function body), but since it has already been cached, it will never be added to
the ASTContext. The solution is to skip verification and avoid caching
it. Instead, loaded modules are now verified in WMO mode once during
performWholeModuleTypeChecking.

Fixes <rdar:36801676> [Edge][swift 4.1] SIL verification failed: external
declarations of SILFunctions with shared visibility is not allowed.
@atrick
Copy link
Contributor Author

atrick commented Jan 29, 2018

I'm still working on a test case. I understand the problem but don't fully understand causality.

@atrick
Copy link
Contributor Author

atrick commented Jan 29, 2018

@swift-ci test.

@atrick atrick requested a review from jrose-apple January 29, 2018 23:35
Copy link
Contributor

@jrose-apple jrose-apple left a comment

Choose a reason for hiding this comment

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

LGTM. If we want to reproduce the original test case, it's going to involve a library module and a client module.

@swift-ci
Copy link
Contributor

Build failed
Swift Test Linux Platform
Git Sha - deebe8b

@swift-ci
Copy link
Contributor

Build failed
Swift Test OS X Platform
Git Sha - deebe8b

… is fixed.

playground_lvalues.swift:
use of unresolved identifier '$builtin_log_with_id'

Additional verification could expose this problem elsewhere. It will be enable
on master later, but not as part of this PR.
@atrick
Copy link
Contributor Author

atrick commented Jan 30, 2018

@swift-ci please test and merge.

@atrick
Copy link
Contributor Author

atrick commented Jan 30, 2018

@swift-ci test and merge.

@swift-ci swift-ci merged commit 2965c8f into swiftlang:master Jan 30, 2018
@atrick atrick deleted the fix-wmo-import branch May 9, 2018 23:50
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