[ConstraintSystem] Deplay opening generic requirements until after co… #25053

xedin · 2019-05-24T19:06:17Z

…ntextual self has been applied

While computing a type of member via getTypeOfMemberReference
let's delay opening generic requirements associated with function
type until after self constraint has been created, that would give
a chance for contextual types to get propagated and make mismatch
originated in generic requirements much easier to diagnose.

Consider following example:

struct S<T> {}

extension S where T == Int {
  func foo() {}
}

func test(_ s: S<String>) {
  s.foo()
}

foo would get opened as (S<$T>) -> () -> Void and contextual self
type is going to be S<String>, so applying that before generic requirement
$T == Int would make sure that $T gets bound to a contextual
type of String and later fails requirement constraint $T == Int.

This is much easier to diagnose comparing to $T being bound to
Int right away due to same-type generic requirement and then
failing an attempt to convert S<String> to S<Int> while simplifying
self constraint.

Resolves: rdar://problem/46427500
Resolves: rdar://problem/34770265

…ntextual self has been applied While computing a type of member via `getTypeOfMemberReference` let's delay opening generic requirements associated with function type until after self constraint has been created, that would give a chance for contextual types to get propagated and make mismatch originated in generic requirements much easier to diagnose. Consider following example: ```swift struct S<T> {} extension S where T == Int { func foo() {} } func test(_ s: S<String>) { s.foo() } ``` `foo` would get opened as `(S<$T>) -> () -> Void` and contextual `self` type is going to be `S<String>`, so applying that before generic requirement `$T == Int` would make sure that `$T` gets bound to a contextual type of `String` and later fails requirement constraint `$T == Int`. This is much easier to diagnose comparing to `$T` being bound to `Int` right away due to same-type generic requirement and then failing an attempt to convert `S<String>` to `S<Int>` while simplifying self constraint. Resolves: rdar://problem/46427500 Resolves: rdar://problem/34770265

slavapestov · 2019-05-24T19:22:26Z

lib/Sema/ConstraintSystem.cpp

@@ -1110,7 +1112,8 @@ void ConstraintSystem::openGeneric(
       GenericSignature *sig,
       bool skipProtocolSelfConstraint,
       ConstraintLocatorBuilder locator,
-       OpenedTypeMap &replacements) {
+       OpenedTypeMap &replacements,
+       bool skipGenericRequirements) {


Instead of adding a new boolean parameter here, how about removing the openGenericRequirements() call below, and then changing all callers to either call it or not? It would be clearer than yet another boolean. This allows you to remove the skipProtocolSelfConstraint parameter from openGeneric() as well, since its only used by openGenericRequirements().

I have considered doing that, but I didn't go that route because it means that we'd have to do duplicate direct calls everywhere when it's only useful in this special case. I think it makes this whole setup more error-prone. Other places like CRanking/getTypeOfReference shouldn't have to do anything special besides a call to openFunctionType/openGeneric.

But there aren't that many calls overall are there? I'm just not a fan of boolean flags as parameters to functions.

You could also factor into three functions, openGeneric() that does both, openGenericParams() creating the type variables and binding the outer archetypes only, and openGenericRequirements()

There aren't many calls at the moment, but I want to make sure that openGeneric/openFunctionType still does the right thing by default. I can absolutely look into refactoring this some more though.

xedin · 2019-05-24T19:49:43Z

@swift-ci please smoke test compiler performance

swift-ci · 2019-05-24T21:38:52Z

Summary for master smoketest

Unexpected test results, excluded stats for ReactiveCocoa

No regressions above thresholds

Debug
- debug brief
- debug detailed
Release
- release brief
- release detailed

Debug

debug brief

Regressed (0)

name	old	new	delta	delta_pct

Improved (1)

name	old	new	delta	delta_pct
time.swift-driver.wall	13.4s	13.1s	-249.9ms	-1.87% ✅

Unchanged (delta < 1.0% or delta < 100.0ms) (2)

name	old	new	delta	delta_pct
Frontend.NumInstructionsExecuted	101,439,028,479	101,123,904,629	-315,123,850	-0.31%
LLVM.NumLLVMBytesOutput	6,147,424	6,147,424	0	0.0%

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) (22)

name	old	new	delta	delta_pct
AST.NumImportedExternalDefinitions	1,028	1,028	0	0.0%
AST.NumLoadedModules	828	828	0	0.0%
AST.NumTotalClangImportedEntities	3,932	3,932	0	0.0%
IRModule.NumIRBasicBlocks	18,752	18,752	0	0.0%
IRModule.NumIRFunctions	10,522	10,522	0	0.0%
IRModule.NumIRGlobals	8,019	8,019	0	0.0%
IRModule.NumIRInsts	313,670	313,670	0	0.0%
IRModule.NumIRValueSymbols	17,602	17,602	0	0.0%
LLVM.NumLLVMBytesOutput	6,147,424	6,147,424	0	0.0%
SILModule.NumSILGenFunctions	5,370	5,370	0	0.0%
SILModule.NumSILOptFunctions	7,016	7,016	0	0.0%
Sema.NumConformancesDeserialized	14,719	14,719	0	0.0%
Sema.NumConstraintScopes	38,682	38,658	-24	-0.06%
Sema.NumDeclsDeserialized	131,831	131,831	0	0.0%
Sema.NumDeclsValidated	11,074	11,074	0	0.0%
Sema.NumFunctionsTypechecked	1,868	1,868	0	0.0%
Sema.NumGenericSignatureBuilders	4,190	4,190	0	0.0%
Sema.NumLazyGenericEnvironments	29,111	29,111	0	0.0%
Sema.NumLazyGenericEnvironmentsLoaded	2,012	2,012	0	0.0%
Sema.NumLazyIterableDeclContexts	22,279	22,279	0	0.0%
Sema.NumTypesDeserialized	56,185	56,185	0	0.0%
Sema.NumTypesValidated	11,858	11,858	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) (3)

name	old	new	delta	delta_pct
Frontend.NumInstructionsExecuted	142,317,719,787	142,242,307,026	-75,412,761	-0.05%
LLVM.NumLLVMBytesOutput	7,155,992	7,155,812	-180	-0.0%
time.swift-driver.wall	23.6s	23.6s	-2.6ms	-0.01%

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) (22)

name	old	new	delta	delta_pct
AST.NumImportedExternalDefinitions	398	398	0	0.0%
AST.NumLoadedModules	76	76	0	0.0%
AST.NumTotalClangImportedEntities	2,196	2,196	0	0.0%
IRModule.NumIRBasicBlocks	21,144	21,144	0	0.0%
IRModule.NumIRFunctions	10,004	10,004	0	0.0%
IRModule.NumIRGlobals	8,031	8,031	0	0.0%
IRModule.NumIRInsts	223,874	223,874	0	0.0%
IRModule.NumIRValueSymbols	17,207	17,207	0	0.0%
LLVM.NumLLVMBytesOutput	7,155,992	7,155,812	-180	-0.0%
SILModule.NumSILGenFunctions	4,176	4,176	0	0.0%
SILModule.NumSILOptFunctions	5,844	5,844	0	0.0%
Sema.NumConformancesDeserialized	12,333	12,333	0	0.0%
Sema.NumConstraintScopes	37,970	37,946	-24	-0.06%
Sema.NumDeclsDeserialized	31,987	31,987	0	0.0%
Sema.NumDeclsValidated	7,784	7,784	0	0.0%
Sema.NumFunctionsTypechecked	1,632	1,632	0	0.0%
Sema.NumGenericSignatureBuilders	1,598	1,598	0	0.0%
Sema.NumLazyGenericEnvironments	6,599	6,599	0	0.0%
Sema.NumLazyGenericEnvironmentsLoaded	130	130	0	0.0%
Sema.NumLazyIterableDeclContexts	4,261	4,261	0	0.0%
Sema.NumTypesDeserialized	17,569	17,569	0	0.0%
Sema.NumTypesValidated	6,942	6,942	0	0.0%

xedin · 2019-05-25T00:00:22Z

@swift-ci please test

xedin · 2019-05-25T00:01:21Z

@swift-ci please test source compatibility

swift-ci · 2019-05-25T01:59:48Z

Build failed
Swift Test OS X Platform
Git Sha - 91dbcfd

xedin · 2019-05-25T02:09:39Z

Looks like this also fixed LLDB crash

Align test error message which apple/swift#25053 which improves diagnostics related to generic requirements.

xedin · 2019-05-28T16:50:38Z

apple/swift-lldb#1631

@swift-ci Please test macOS platform

DougGregor

This looks correct and is an improvement, although that set of bool parameters is starting to make me a bit twitchy.

xedin · 2019-05-29T00:14:51Z

@DougGregor I talked to @slavapestov about this offline as well. Going to get this merged and refactor separately as I wanted to get this into 5.1 with minimal changes.

Align test error message which apple/swift#25053 which improves diagnostics related to generic requirements. (cherry picked from commit bbe2050)

xedin requested a review from DougGregor May 24, 2019 19:06

slavapestov reviewed May 24, 2019

View reviewed changes

xedin added a commit to xedin/swift-lldb that referenced this pull request May 28, 2019

[Tests] Adjust error message for failing member lookup test

bbe2050

Align test error message which apple/swift#25053 which improves diagnostics related to generic requirements.

xedin mentioned this pull request May 28, 2019

[Tests] Adjust error message for failing member lookup test apple/swift-lldb#1631

Merged

DougGregor approved these changes May 29, 2019

View reviewed changes

xedin merged commit cfb8d1b into apple:master May 29, 2019

xedin mentioned this pull request May 29, 2019

[SR-9396] Nonsensical error message related to constrained extensions #51862

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[ConstraintSystem] Deplay opening generic requirements until after co… #25053

[ConstraintSystem] Deplay opening generic requirements until after co… #25053

xedin commented May 24, 2019

slavapestov May 24, 2019

xedin May 24, 2019

slavapestov May 24, 2019

slavapestov May 24, 2019

xedin May 24, 2019

xedin commented May 24, 2019

swift-ci commented May 24, 2019

Debug

debug brief

debug detailed

Release

release brief

release detailed

xedin commented May 25, 2019

xedin commented May 25, 2019

swift-ci commented May 25, 2019

xedin commented May 25, 2019

xedin commented May 28, 2019

DougGregor left a comment

xedin commented May 29, 2019

[ConstraintSystem] Deplay opening generic requirements until after co… #25053

[ConstraintSystem] Deplay opening generic requirements until after co… #25053

Conversation

xedin commented May 24, 2019

slavapestov May 24, 2019

Choose a reason for hiding this comment

xedin May 24, 2019

Choose a reason for hiding this comment

slavapestov May 24, 2019

Choose a reason for hiding this comment

slavapestov May 24, 2019

Choose a reason for hiding this comment

xedin May 24, 2019

Choose a reason for hiding this comment

xedin commented May 24, 2019

swift-ci commented May 24, 2019

Summary for master smoketest

Debug

debug brief

debug detailed

Release

release brief

release detailed

xedin commented May 25, 2019

xedin commented May 25, 2019

swift-ci commented May 25, 2019

xedin commented May 25, 2019

xedin commented May 28, 2019

DougGregor left a comment

Choose a reason for hiding this comment

xedin commented May 29, 2019