Skip to content

[Diagnostics] Correctly identify location of requirement failure #26677

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 4 commits into from
Aug 17, 2019
Merged

[Diagnostics] Correctly identify location of requirement failure #26677

merged 4 commits into from
Aug 17, 2019

Conversation

xedin
Copy link
Contributor

@xedin xedin commented Aug 15, 2019

Previously in situations like:

protocol P {}

struct S<T: P> {
  var value: T
}

_ = S(value: 42)

Diagnostic has reported a problem as related to "reference" to init
but the failing generic type requirement belongs to S, so a
better diagnostic in such case should mention generic struct S.

In order to do that we need to first de-duplicate generic type requirement
checking since constraint system would record generic parameter multiple
times: 1. While resolving S and 2. When determining a type of constructor member.

And afterwards add special logic to RequirementFailure to determine where
generic signature came from.

@xedin xedin requested review from DougGregor and hborla August 15, 2019 21:05
@xedin
Copy link
Contributor Author

xedin commented Aug 15, 2019

@swift-ci please test source compatibility

@xedin
Copy link
Contributor Author

xedin commented Aug 15, 2019

@swift-ci please smoke test compiler performance

@swift-ci
Copy link
Contributor

Summary for master smoketest

Unexpected test results, excluded stats for 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) (3)
name old new delta delta_pct
Frontend.NumInstructionsExecuted 100,503,837,376 100,526,970,181 23,132,805 0.02%
LLVM.NumLLVMBytesOutput 6,145,872 6,145,876 4 0.0%
time.swift-driver.wall 8.1s 8.0s -63.6ms -0.79%

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) (21)
name old new delta delta_pct
AST.NumLoadedModules 660 660 0 0.0%
AST.NumTotalClangImportedEntities 3,768 3,768 0 0.0%
IRModule.NumIRBasicBlocks 17,905 17,905 0 0.0%
IRModule.NumIRFunctions 10,613 10,613 0 0.0%
IRModule.NumIRGlobals 8,320 8,320 0 0.0%
IRModule.NumIRInsts 308,861 308,861 0 0.0%
IRModule.NumIRValueSymbols 17,957 17,957 0 0.0%
LLVM.NumLLVMBytesOutput 6,145,872 6,145,876 4 0.0%
SILModule.NumSILGenFunctions 5,372 5,372 0 0.0%
SILModule.NumSILOptFunctions 7,270 7,270 0 0.0%
Sema.NumConformancesDeserialized 11,824 11,824 0 0.0%
Sema.NumConstraintScopes 38,612 38,612 0 0.0%
Sema.NumDeclsDeserialized 113,400 113,400 0 0.0%
Sema.NumDeclsValidated 9,600 9,600 0 0.0%
Sema.NumFunctionsTypechecked 2,034 2,034 0 0.0%
Sema.NumGenericSignatureBuilders 4,086 4,086 0 0.0%
Sema.NumLazyGenericEnvironments 22,698 22,698 0 0.0%
Sema.NumLazyGenericEnvironmentsLoaded 1,942 1,942 0 0.0%
Sema.NumLazyIterableDeclContexts 18,481 18,481 0 0.0%
Sema.NumTypesDeserialized 44,023 44,023 0 0.0%
Sema.NumTypesValidated 8,196 8,196 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 141,994,615,775 142,065,215,117 70,599,342 0.05%
LLVM.NumLLVMBytesOutput 7,050,480 7,050,468 -12 -0.0%
time.swift-driver.wall 16.1s 16.0s -3.8ms -0.02%

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) (21)
name old new delta delta_pct
AST.NumLoadedModules 76 76 0 0.0%
AST.NumTotalClangImportedEntities 2,206 2,206 0 0.0%
IRModule.NumIRBasicBlocks 18,887 18,887 0 0.0%
IRModule.NumIRFunctions 10,141 10,141 0 0.0%
IRModule.NumIRGlobals 8,346 8,346 0 0.0%
IRModule.NumIRInsts 209,402 209,402 0 0.0%
IRModule.NumIRValueSymbols 17,667 17,667 0 0.0%
LLVM.NumLLVMBytesOutput 7,050,480 7,050,468 -12 -0.0%
SILModule.NumSILGenFunctions 4,178 4,178 0 0.0%
SILModule.NumSILOptFunctions 6,226 6,226 0 0.0%
Sema.NumConformancesDeserialized 13,423 13,423 0 0.0%
Sema.NumConstraintScopes 38,438 38,438 0 0.0%
Sema.NumDeclsDeserialized 31,993 31,993 0 0.0%
Sema.NumDeclsValidated 7,720 7,720 0 0.0%
Sema.NumFunctionsTypechecked 2,034 2,034 0 0.0%
Sema.NumGenericSignatureBuilders 1,601 1,601 0 0.0%
Sema.NumLazyGenericEnvironments 6,238 6,238 0 0.0%
Sema.NumLazyGenericEnvironmentsLoaded 130 130 0 0.0%
Sema.NumLazyIterableDeclContexts 4,279 4,279 0 0.0%
Sema.NumTypesDeserialized 18,249 18,249 0 0.0%
Sema.NumTypesValidated 5,108 5,108 0 0.0%

@xedin
Copy link
Contributor Author

xedin commented Aug 16, 2019

@swift-ci please smoke test

DougGregor
DougGregor approved these changes Aug 16, 2019
View reviewed changes
xedin added 4 commits August 16, 2019 22:02
@xedin
[ConstraintSystem] Deduplicate generic requirements after merging typ…
661f636 
…e variables

This comes up, for example, when `Self` type parameter is bound to its
argument in a member reference, both argument and parameter can refer
to exact same requirements, that have to be deduplicated otherwise
they are just going to get "fixed" N times, which complicates diagnostics.
@xedin
[ConstraintSystem] Track all generic requirements fixed along current…
12648d6 
… path

Such tracking makes it easier to ignore already "fixed" requirements
which have been recorded in the constraint system multiple times e.g.
a call to initializer would open both base type and initializer
method which have shared (if not the same) requirements.
@xedin
[Diagnostics] Correctly identify location of requirement failure
cbadd7f 
Previously in situations like:

```swift
protocol P {}

struct S<T: P> {
  var value: T
}

_ = S(value: 42)
```

Diagnostic has reported a problem as related to "reference" to `init`
but the failing generic type requirement belongs to `S`, so a
better diagnostic in such case should mention `generic struct S`.
@xedin
[ContraintSystem] Don't de-duplicate generic requirements
df47d1b 
Looks like this is not really needed since fixed requirements
cache can handle such de-duplication.
@xedin xedin force-pushed the dedup-generic-requirements branch from c15b8df to df47d1b Compare August 17, 2019 05:23
@xedin
Copy link
Contributor Author

xedin commented Aug 17, 2019

@swift-ci please smoke test

@xedin xedin merged commit fc3d975 into swiftlang:master Aug 17, 2019
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

@hborla hborla hborla 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.
4 participants
@xedin @swift-ci @DougGregor @hborla