Skip to content

A Raft of Fixes To Private Dependencies #32198

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
Jun 5, 2020
Merged

Conversation

CodaFi
Copy link
Contributor

@CodaFi CodaFi commented Jun 5, 2020

  • Turn the dependency verifier loose on the entire compiler pipeline, not just the typechecker
  • Fixup the getter for the active dependency source. It was mistakenly set to read the source stack the old way when I refactored this in Delete ReferencedNameTracker #31960
  • Make LookupInModuleRequest a dependency sink
  • Commit a tricky regression test that verifies we always take a dependency on the underlying type of a typealias.
  • Perform the unioning step during request replay.

Resolves rdar://64008262

CodaFi added 2 commits June 4, 2020 18:27
The old tests were just running the type checker because we used to only emit reference dependencies after Sema. Now that we emit them after the pipeline has run, let's upgrade these tests to capture these new references.
Restore the behavior here of only looking at the topmost dependency source object that was lost when the referenced name tracker was removed.

Oops.
@CodaFi CodaFi requested a review from slavapestov June 5, 2020 01:41
@CodaFi
Copy link
Contributor Author

CodaFi commented Jun 5, 2020

@swift-ci smoke test

CodaFi added 2 commits June 4, 2020 20:17
It wasn't one in the past because the referenced name tracker was written into by unqualified lookup, which would delegate to this request. Now that it's no longer doing that, we have to capture this edge here for private dependencies.
@CodaFi CodaFi force-pushed the depen-dots branch 2 times, most recently from 873554f to 561cc6f Compare June 5, 2020 03:30
In order for private dependencies to be completely correct, it must perform the name lookup unioning step when a cached request is replayed - not just when lookups are first performed. In order to reduce the overhead of this union operation, it is not necessary to walk the entire active request stack, just walk to the nearest cached request in the stack and union into that. When it is popped, its replay step will itself union into the next cached request.

To see why, consider a request graph:

A* -> B -> C*
         |
          -> D*

where A, C, and D are cached.

If a caller were to force C and D, then force A independenty, today we would *only* replay the names looked up by C and D the first time A was evaluated. That is, subsequent evaluations of A do not replay the correct set of names. If we were to perform the union step during replay as well, requests that force A would also see C and D’s lookups.

Without this, callers that force requests like the DeclChecker have to be wary of the way they force the interface type request so other files see the right name sets.

rdar://64008262
@CodaFi
Copy link
Contributor Author

CodaFi commented Jun 5, 2020

@swift-ci smoke test

@CodaFi
Copy link
Contributor Author

CodaFi commented Jun 5, 2020

Just to see what happens here

@swift-ci test compiler performance

@CodaFi
Copy link
Contributor Author

CodaFi commented Jun 5, 2020

(Oops, let's cancel the perf test. I meant to push a different stack of commits before that).

@swift-ci
Copy link
Contributor

swift-ci commented Jun 5, 2020

Compilation-performance test failed

@CodaFi
Copy link
Contributor Author

CodaFi commented Jun 5, 2020

@CodaFi CodaFi merged commit 31cf1f2 into swiftlang:master Jun 5, 2020
@CodaFi CodaFi deleted the depen-dots branch June 5, 2020 22:18
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