Skip to content

Preserve specialized conformance better #17993

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
Jul 17, 2018
Merged

Preserve specialized conformance better #17993

merged 3 commits into from
Jul 17, 2018

Conversation

slavapestov
Copy link
Contributor

In a couple of places we would collapse a specialized conformance of a type down to its normal conformance if the substituted type was equal to the original type. But this is not correct, because even if every generic parameter in a generic signature is mapped to itself, a conformance requirement might map to a concrete conformance and not an abstract conformance.

Fixes rdar://problem/40164371.

@slavapestov
Copy link
Contributor Author

@swift-ci Please smoke test

@slavapestov
Copy link
Contributor Author

@swift-ci Please test source compatibility

@slavapestov
Copy link
Contributor Author

@swift-ci Please smoke test compiler performance

@slavapestov slavapestov mentioned this pull request Jul 16, 2018
Merged
DougGregor
DougGregor approved these changes Jul 17, 2018
View reviewed changes
Copy link
Member

@DougGregor DougGregor left a comment

Choose a reason for hiding this comment

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

This is quite clean, thank you!

@swift-ci
Copy link
Contributor

Build comment file:

Summary for master smoketest

Unexpected test results, excluded stats for Kingfisher, ReactiveCocoa

No regressions above thresholds

Debug

debug brief

Regressed (0)
name old new delta delta_pct
Improved (0)
name old new delta delta_pct
Unchanged (delta < 1.0% or delta < 100.0ms) (2)
name old new delta delta_pct
LLVM.NumLLVMBytesOutput 9,380,524 9,380,524 0 0.0%
time.swift-driver.wall 22.1s 22.3s 149.3ms 0.67%

debug detailed

Regressed (0)
name old new delta delta_pct
Improved (0)
name old new delta delta_pct
Unchanged (delta < 1.0% or delta < 100.0ms) (23)
name old new delta delta_pct
AST.NumImportedExternalDefinitions 5,979 5,979 0 0.0%
AST.NumLoadedModules 1,510 1,510 0 0.0%
AST.NumTotalClangImportedEntities 17,688 17,688 0 0.0%
AST.NumUsedConformances 1,289 1,289 0 0.0%
IRModule.NumIRBasicBlocks 31,156 31,156 0 0.0%
IRModule.NumIRFunctions 17,189 17,189 0 0.0%
IRModule.NumIRGlobals 14,007 14,007 0 0.0%
IRModule.NumIRInsts 437,617 437,617 0 0.0%
IRModule.NumIRValueSymbols 29,066 29,066 0 0.0%
LLVM.NumLLVMBytesOutput 9,380,524 9,380,524 0 0.0%
SILModule.NumSILGenFunctions 8,037 8,037 0 0.0%
SILModule.NumSILOptFunctions 10,970 10,970 0 0.0%
Sema.NumConformancesDeserialized 43,402 43,402 0 0.0%
Sema.NumConstraintScopes 75,646 75,646 0 0.0%
Sema.NumDeclsDeserialized 274,691 274,691 0 0.0%
Sema.NumDeclsValidated 27,827 27,827 0 0.0%
Sema.NumFunctionsTypechecked 4,974 4,974 0 0.0%
Sema.NumGenericSignatureBuilders 12,532 12,532 0 0.0%
Sema.NumLazyGenericEnvironments 53,702 53,702 0 0.0%
Sema.NumLazyGenericEnvironmentsLoaded 6,287 6,287 0 0.0%
Sema.NumLazyIterableDeclContexts 42,156 42,156 0 0.0%
Sema.NumTypesDeserialized 294,174 294,174 0 0.0%
Sema.NumTypesValidated 47,653 47,653 0 0.0%

Release

release brief

Regressed (0)
name old new delta delta_pct
Improved (0)
name old new delta delta_pct
Unchanged (delta < 1.0% or delta < 100.0ms) (2)
name old new delta delta_pct
LLVM.NumLLVMBytesOutput 10,591,500 10,591,500 0 0.0%
time.swift-driver.wall 36.5s 36.4s -123.2ms -0.34%

release detailed

Regressed (0)
name old new delta delta_pct
Improved (0)
name old new delta delta_pct
Unchanged (delta < 1.0% or delta < 100.0ms) (23)
name old new delta delta_pct
AST.NumImportedExternalDefinitions 1,400 1,400 0 0.0%
AST.NumLoadedModules 100 100 0 0.0%
AST.NumTotalClangImportedEntities 4,948 4,948 0 0.0%
AST.NumUsedConformances 1,292 1,292 0 0.0%
IRModule.NumIRBasicBlocks 34,605 34,605 0 0.0%
IRModule.NumIRFunctions 15,426 15,426 0 0.0%
IRModule.NumIRGlobals 13,862 13,862 0 0.0%
IRModule.NumIRInsts 341,907 341,907 0 0.0%
IRModule.NumIRValueSymbols 27,522 27,522 0 0.0%
LLVM.NumLLVMBytesOutput 10,591,500 10,591,500 0 0.0%
SILModule.NumSILGenFunctions 6,180 6,180 0 0.0%
SILModule.NumSILOptFunctions 9,527 9,527 0 0.0%
Sema.NumConformancesDeserialized 19,764 19,764 0 0.0%
Sema.NumConstraintScopes 74,238 74,238 0 0.0%
Sema.NumDeclsDeserialized 57,216 57,216 0 0.0%
Sema.NumDeclsValidated 20,774 20,774 0 0.0%
Sema.NumFunctionsTypechecked 3,079 3,079 0 0.0%
Sema.NumGenericSignatureBuilders 2,846 2,846 0 0.0%
Sema.NumLazyGenericEnvironments 9,650 9,650 0 0.0%
Sema.NumLazyGenericEnvironmentsLoaded 1,361 1,361 0 0.0%
Sema.NumLazyIterableDeclContexts 5,723 5,723 0 0.0%
Sema.NumTypesDeserialized 72,284 72,284 0 0.0%
Sema.NumTypesValidated 22,840 22,840 0 0.0%

@slavapestov
Copy link
Contributor Author

Oops

@slavapestov slavapestov reopened this Jul 17, 2018
@slavapestov
AST: Relax an assertion
1ff91fe
@slavapestov
Sema: Simplify GatherConformancesListener
f242a23 
The inherited conformance check was dead because the type of a
conformance can never be an archetype.

Also, take care to substitute the conformance we are given, instead
of throwing it out and looking up a new one, because we want to be
careful to preserve any existing substitutions.

Part of the fix for <rdar://problem/40164371>.
@slavapestov
AST: Fix over-eager collapseSpecializedConformance()
42a3ad3 
It's possible that the conforming type is equal to the generic
conformance type, but some of the substitutions replace an
abstract conformance with a concrete one.

In this case we cannot collapse away the specialized conformance,
because we lose information that way.

Fixes <rdar://problem/40164371>.
@slavapestov slavapestov force-pushed the preserve-specialized-conformance-better branch from 4cc2d1b to 42a3ad3 Compare July 17, 2018 21:42
@slavapestov
Copy link
Contributor Author

@swift-ci Please smoke test

@slavapestov slavapestov merged commit 86c26a9 into swiftlang:master Jul 17, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@DougGregor DougGregor DougGregor approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@slavapestov @swift-ci @DougGregor