-
Notifications
You must be signed in to change notification settings - Fork 10.5k
[Serialization] Encode depth for cross-refs to generic parameters #20091
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
Conversation
This file is very specifically counting the number of declarations that get deserialized under a certain scenario, and so it's not really the place to add tests for other things about cross-references and extensions.
Otherwise, we can't represent a cross-reference to generic parameters in a parent type /when used in an extension/. https://bugs.swift.org/browse/SR-9084
@swift-ci Please test |
|
@swift-ci Please test source compatibility |
@swift-ci Please test compiler performance |
} | ||
while (currentDepth > depth) { | ||
paramList = paramList->getOuterParameters(); | ||
--currentDepth; |
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.
Is it worth asserting that the GenericTypeParamDecl you find below has the right depth and index?
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.
That seems like a good idea for the AST verifier in general. It's set up by construction that the depth matches the GenericParamList's notion of depth, but the GenericParamList and the GenericTypeParamDecl could be out of sync. I'll do that in a follow-up PR.
These compiler perf numbers are all wrong again. @graydon, worth investigating, or should I just move on? |
@jrose-apple I've pushed a fix that ought to give better detection of failed modules. If you don't mind, I'd like to run the same cperf test again? |
@swift-ci Please test compiler performance |
Build comment file:Summary for master fullUnexpected test results, excluded stats for NonEmpty, RxSwift, Wordy, GRDB, ReactiveSwift Regressions found (see below) Debug-batchdebug-batch briefRegressed (3)
Improved (0)
Unchanged (delta < 1.0% or delta < 100.0ms) (0)
debug-batch detailedRegressed (52)
Improved (0)
Unchanged (delta < 1.0% or delta < 100.0ms) (43)
Releaserelease briefRegressed (0)
Improved (1)
Unchanged (delta < 1.0% or delta < 100.0ms) (2)
release detailedRegressed (0)
Improved (0)
Unchanged (delta < 1.0% or delta < 100.0ms) (23)
|
Hm, not quite there yet… Is the script only detecting when the new compiler fails? This is a change where it used to fail and now it succeeds. |
@jrose-apple it's trying to figure out which modules failed in either instance (old or new) so it can exclude them from comparison. I .. do not understand what happened here. Sigh. Will try again. |
@jrose-apple one more time! |
@swift-ci Please test compiler performance |
@jrose-apple @graydon Going to have to kill compiler performance job, currently updating Xcode. |
Build comment file:Compilation-performance test failed |
@swift-ci Please test compiler performance |
Build comment file:Summary for master fullUnexpected test results, excluded stats for NonEmpty, ReactiveExtensions_TestHelpers, ProcedureKitCloud, GRDB, ReactiveSwift, ReactiveCocoa, Wordy, ReactiveExtensions No regressions above thresholds Debug-batchdebug-batch briefRegressed (0)
Improved (0)
Unchanged (delta < 1.0% or delta < 100.0ms) (3)
debug-batch detailedRegressed (0)
Improved (0)
Unchanged (delta < 1.0% or delta < 100.0ms) (95)
Releaserelease briefRegressed (0)
Improved (0)
Unchanged (delta < 1.0% or delta < 100.0ms) (3)
release detailedRegressed (0)
Improved (0)
Unchanged (delta < 1.0% or delta < 100.0ms) (23)
|
There we go! Merging. |
@jrose-apple thanks for your perseverance, I know it's tedious and in a better world we'd have set all this up to be much more directly testable :( |
Otherwise, we can't represent a cross-reference to generic parameters in a parent type when used in an extension.
SR-9084 / rdar://problem/45566043