[CSBindings] Cleanup literal binding inference #15489

xedin · 2018-03-24T18:48:18Z

Don't attempt to store literal bindings directly to PotentialBindings
since they might get superseded by non-literal bindings deduced from
other constraints, also don't attempt to check literal protocol conformance
on type variables or member types since such types would always end-up
returning trivial conformance which results in removal of viable literal types.

Resolves: rdar://problem/38535743

xedin · 2018-03-24T18:48:47Z

@swift-ci please test source compatibility

xedin · 2018-03-24T18:51:43Z

In the expressions like:

var foo: String? = "hello"
public func bar<T : Equatable>(_ a: @autoclosure () throws -> T, _ b: @autoclosure () throws -> T) {}

bar(foo, "Yes")

We end up loosing String type for right-hand side type variable and only end up with type variable from foo but resulting bindings would still have literal kind set which is not ideal, although still leads to a solution based on foo currently, but if the order was to change, bar(foo, "Yes") wouldn't type-check.

xedin · 2018-03-24T18:52:26Z

validation-test/Sema/type_checker_perf/slow/nil_coalescing.swift.gyb

@@ -1,4 +1,4 @@
-// RUN: %scale-test --invert-result --begin 1 --end 5 --step 1 --select incrementScopeCounter %s
+// RUN: not %scale-test --invert-result --begin 1 --end 5 --step 1 --select incrementScopeCounter %s


Starting returning "too complex" in different spots, didn't know how to make it work otherwise...

Shouldn't this change instead remove --invert-result and move the test to fast/?

Unfortunately this one didn't improve but now produces "too complex" in one of the chained "+" which I don't know how to mark with expected-error so I added not instead.

Okay, then I think we should open a bug and just disable the test now. It's taking several minutes to run on my machine. You can feel free to merge this and then disable as a separate PR.

Sure, I've created rdar://problem/38963783 and going to merge this all together, there is no rush.

xedin · 2018-03-25T00:31:44Z

Looks like this exposed one more problem with join, but i don’t think we should be joining with literals anyway.

xedin · 2018-03-25T01:53:49Z

@swift-ci please test source compatibility

xedin · 2018-03-25T05:44:47Z

@swift-ci please smoke test compiler performance

swift-ci · 2018-03-25T07:24:52Z

Build comment file:

Summary for master smoketest

Regressions found (see below)

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	80.3s	78.9s	-1.4s	-1.74% ✅

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

name	old	new	delta	delta_pct
LLVM.NumLLVMBytesOutput	47,059,890	47,059,718	-172	-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) (23)

name	old	new	delta	delta_pct
AST.NumImportedExternalDefinitions	113,826	113,826	0	0.0%
AST.NumLoadedModules	16,158	16,158	0	0.0%
AST.NumTotalClangImportedEntities	337,992	337,992	0	0.0%
AST.NumUsedConformances	9,439	9,439	0	0.0%
IRModule.NumIRBasicBlocks	135,839	135,839	0	0.0%
IRModule.NumIRFunctions	80,197	80,197	0	0.0%
IRModule.NumIRGlobals	74,295	74,295	0	0.0%
IRModule.NumIRInsts	1,499,883	1,499,883	0	0.0%
IRModule.NumIRValueSymbols	137,667	137,667	0	0.0%
LLVM.NumLLVMBytesOutput	47,059,890	47,059,718	-172	-0.0%
SILModule.NumSILGenFunctions	49,962	49,962	0	0.0%
SILModule.NumSILOptFunctions	90,815	90,815	0	0.0%
Sema.NumConformancesDeserialized	469,963	469,887	-76	-0.02%
Sema.NumConstraintScopes	1,018,447	1,022,907	4,460	0.44%
Sema.NumDeclsDeserialized	3,049,337	3,049,252	-85	-0.0%
Sema.NumDeclsValidated	162,677	162,677	0	0.0%
Sema.NumFunctionsTypechecked	64,396	64,396	0	0.0%
Sema.NumGenericSignatureBuilders	85,496	85,496	0	0.0%
Sema.NumLazyGenericEnvironments	606,643	606,636	-7	-0.0%
Sema.NumLazyGenericEnvironmentsLoaded	50,032	50,032	0	0.0%
Sema.NumLazyIterableDeclContexts	439,448	439,378	-70	-0.02%
Sema.NumTypesDeserialized	3,287,176	3,287,147	-29	-0.0%
Sema.NumTypesValidated	272,637	273,600	963	0.35%

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	38,455,136	38,455,133	-3	-0.0%
time.swift-driver.wall	176.4s	176.3s	-159.2ms	-0.09%

release detailed

Regressed (1)

name	old	new	delta	delta_pct
Sema.NumTypesValidated	44,130	45,099	969	2.2% ⛔

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	11,354	11,354	0	0.0%
AST.NumLoadedModules	462	462	0	0.0%
AST.NumTotalClangImportedEntities	32,031	32,031	0	0.0%
AST.NumUsedConformances	9,444	9,444	0	0.0%
IRModule.NumIRBasicBlocks	177,477	177,477	0	0.0%
IRModule.NumIRFunctions	61,660	61,660	0	0.0%
IRModule.NumIRGlobals	60,177	60,177	0	0.0%
IRModule.NumIRInsts	1,479,334	1,479,334	0	0.0%
IRModule.NumIRValueSymbols	110,141	110,141	0	0.0%
LLVM.NumLLVMBytesOutput	38,455,136	38,455,133	-3	-0.0%
SILModule.NumSILGenFunctions	25,070	25,070	0	0.0%
SILModule.NumSILOptFunctions	42,654	42,654	0	0.0%
Sema.NumConformancesDeserialized	157,586	157,586	0	0.0%
Sema.NumConstraintScopes	876,836	881,302	4,466	0.51%
Sema.NumDeclsDeserialized	258,235	258,234	-1	-0.0%
Sema.NumDeclsValidated	31,065	31,065	0	0.0%
Sema.NumFunctionsTypechecked	12,616	12,616	0	0.0%
Sema.NumGenericSignatureBuilders	7,675	7,675	0	0.0%
Sema.NumLazyGenericEnvironments	39,816	39,816	0	0.0%
Sema.NumLazyGenericEnvironmentsLoaded	5,035	5,035	0	0.0%
Sema.NumLazyIterableDeclContexts	24,644	24,643	-1	-0.0%
Sema.NumTypesDeserialized	316,278	316,278	0	0.0%

xedin · 2018-03-26T03:43:58Z

@swift-ci please test source compatibility

xedin · 2018-03-26T08:17:58Z

@swift-ci please clean test

swift-ci · 2018-03-26T10:40:22Z

Build failed
Swift Test OS X Platform
Git Sha - 0ab8721025cda508d92538828b09416a9ea6983d

xedin · 2018-03-26T18:11:04Z

Test FunctionalTests.MiscellaneousTestCase testSecondBuildIsNullInModulemapGen has failed, but it doesn't look like it's caused by these changes, maybe just unstable, I'll re-run.

xedin · 2018-03-26T18:11:16Z

@swift-ci please test macOS platform

rudkx · 2018-03-26T22:11:58Z

lib/Sema/CSBindings.cpp

-      !binding.BindingType->hasTypeVariable() && !binding.DefaultedProtocol &&
-      !binding.isDefaultableBinding() && allowJoinMeet) {
+      !binding.BindingType->hasTypeVariable() &&
+      !binding.BindingType->hasUnboundGenericType() &&


It looks like the diff here is checking for hasUnboundGenericType().

It's not clear to me why we would have to do that here. It's possible join was hitting an llvm_unreachable, but I recently fixed that to return Unimplemented in the cases we aren't currently handling - it was an oversight that I checked in the llvm_unreachable.

I've added that because Kickstarter-Prelude has hitting "non-canonical or unchecked type" llvm_unreachable which trying to join Array and [String]? since Array is 'UnboundGenericType` which is "unchecked".

rudkx · 2018-03-28T05:39:20Z

test/Constraints/rdar38535743.swift

+// RUN: %target-typecheck-verify-swift
+// REQUIRES: objc_interop
+
+import Foundation


This should use the mock SDK. Grep for 'mock-sdk' e.g. in tests/Constraints/*_objc.swift.

Don't attempt to store literal bindings directly to `PotentialBindings` since they might get superseded by non-literal bindings deduced from other constraints, also don't attempt to check literal protocol conformance on type variables or member types since such types would always end-up returning trivial conformance which results in removal of viable literal types. Resolves: rdar://problem/38535743

xedin · 2018-03-28T06:32:36Z

@swift-ci please clean test

…38963783)

xedin · 2018-03-28T18:20:07Z

@swift-ci please smoke test

xedin requested a review from rudkx March 24, 2018 18:48

xedin commented Mar 24, 2018

View reviewed changes

xedin force-pushed the clean-literal-bindings branch from 22e833f to 531a222 Compare March 25, 2018 01:52

xedin mentioned this pull request Mar 25, 2018

[CSBindings] Delay binding type variables which are dependent on other constraints #15457

Closed

xedin force-pushed the clean-literal-bindings branch from 531a222 to 0ab8721 Compare March 26, 2018 03:42

rudkx reviewed Mar 26, 2018

View reviewed changes

rudkx reviewed Mar 28, 2018

View reviewed changes

xedin force-pushed the clean-literal-bindings branch from 0ab8721 to 298bf2a Compare March 28, 2018 06:31

swiftlang deleted a comment from swift-ci Mar 28, 2018

[Sema] Disable long running nil coalescing perf test (rdar://problem/…

df1cb55

…38963783)

xedin merged commit 31907d4 into swiftlang:master Mar 28, 2018

slavapestov mentioned this pull request Apr 10, 2018

[SR-3849] Combined class and protocol constraints aren't propagated correctly #46434

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[CSBindings] Cleanup literal binding inference #15489

[CSBindings] Cleanup literal binding inference #15489

xedin commented Mar 24, 2018 •

edited

Loading

xedin commented Mar 24, 2018

xedin commented Mar 24, 2018

xedin Mar 24, 2018

rudkx Mar 26, 2018

xedin Mar 26, 2018

rudkx Mar 28, 2018

xedin Mar 28, 2018

xedin commented Mar 25, 2018

xedin commented Mar 25, 2018

xedin commented Mar 25, 2018

swift-ci commented Mar 25, 2018

Debug

debug brief

debug detailed

Release

release brief

release detailed

xedin commented Mar 26, 2018

xedin commented Mar 26, 2018

swift-ci commented Mar 26, 2018

xedin commented Mar 26, 2018

xedin commented Mar 26, 2018

rudkx Mar 26, 2018

xedin Mar 26, 2018

rudkx Mar 28, 2018

xedin Mar 28, 2018

xedin commented Mar 28, 2018

xedin commented Mar 28, 2018

		@@ -1,4 +1,4 @@
		// RUN: %scale-test --invert-result --begin 1 --end 5 --step 1 --select incrementScopeCounter %s
		// RUN: not %scale-test --invert-result --begin 1 --end 5 --step 1 --select incrementScopeCounter %s

[CSBindings] Cleanup literal binding inference #15489

[CSBindings] Cleanup literal binding inference #15489

Conversation

xedin commented Mar 24, 2018 • edited Loading

xedin commented Mar 24, 2018

xedin commented Mar 24, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

xedin commented Mar 25, 2018

xedin commented Mar 25, 2018

xedin commented Mar 25, 2018

swift-ci commented Mar 25, 2018

Build comment file:

Summary for master smoketest

Debug

debug brief

debug detailed

Release

release brief

release detailed

xedin commented Mar 26, 2018

xedin commented Mar 26, 2018

swift-ci commented Mar 26, 2018

xedin commented Mar 26, 2018

xedin commented Mar 26, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

xedin commented Mar 28, 2018

xedin commented Mar 28, 2018

xedin commented Mar 24, 2018 •

edited

Loading