Skip to content

[TypeChecker] Produce a tailored diagnostic for for-in sequence fai… #27834

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

Merged
merged 1 commit into from
Oct 23, 2019

Conversation

xedin
Copy link
Contributor

@xedin xedin commented Oct 22, 2019

…lures

for-in "sequence" expression is required to conform to Sequence.

Resolves: rdar://problem/56559847

@xedin
Copy link
Contributor Author

xedin commented Oct 22, 2019

@swift-ci please smoke test

1 similar comment
@xedin
Copy link
Contributor Author

xedin commented Oct 22, 2019

@swift-ci please smoke test

@@ -15,7 +15,7 @@ struct BadContainer2 : Sequence { // expected-error{{type 'BadContainer2' does n

func bad_containers_2(bc: BadContainer2) {
for e in bc { }
// expected-error@-1{{variable 'e' is not bound by any pattern}}
// expected-warning@-1 {{immutable value 'e' was never used; consider replacing with '_' or removing it}}
Copy link
Contributor

Choose a reason for hiding this comment

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

Why did this and a few others below suddenly become valid?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is because BadContainer2 is declared to be conforming to Sequence, although it's invalid conformance constraint succeeds. It used to fail on the dependent member type associated with sequence element but I made it so that is handled as a hole.

Copy link
Contributor

Choose a reason for hiding this comment

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

Everywhere variable 'e' is not bound by any pattern exists on a non-recursive decl is a sign that we are either not invalidating the AST properly, or we aren't constructing constraint systems with sufficient information to invalidate an associated pattern binding specifically. It's good to see movement here.

// expected-error@-1 {{operator function '..<' requires that 'Lens<Int>' conform to 'Strideable'}}
// expected-error@-2 {{operator function '..<' requires that 'Lens<Int>.Stride' conform to 'SignedInteger'}}
// expected-error@-1 {{protocol 'Sequence' requires that 'Lens<Int>' conform to 'Strideable'}}
// expected-error@-2 {{protocol 'Sequence' requires that 'Lens<Int>.Stride' conform to 'SignedInteger'}}
Copy link
Contributor

Choose a reason for hiding this comment

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

This is technically not accurate, the requirement is not coming from the Sequence protocol itself.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Diagnostics here aren't great because we still don't have a full coverage, but that's something on my radar.

@xedin
Copy link
Contributor Author

xedin commented Oct 22, 2019

@swift-ci please smoke test

…lures

`for-in` "sequence" expression is required to conform to `Sequence`.
@xedin
Copy link
Contributor Author

xedin commented Oct 23, 2019

@swift-ci please smoke test

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants