Skip to content

[ConstraintSystem] Use missing conformance fix to diagnose contextual failures #24754

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
May 17, 2019
Merged

[ConstraintSystem] Use missing conformance fix to diagnose contextual failures #24754

merged 3 commits into from
May 17, 2019

Conversation

xedin
Copy link
Contributor

@xedin xedin commented May 14, 2019

Extend use of missing protocol conformance fix to cover contextual
failures, such as:

  • Assignment mismatches, where destination requires source to conform
    to certain protocol (or protocol composition);
  • Incorrect returns where returned type doesn't conform to the
    protocol specified in the signature.

slavapestov
slavapestov reviewed May 14, 2019
View reviewed changes
@xedin
Copy link
Contributor Author

xedin commented May 14, 2019

@slavapestov This is still a draft, I'm trying to work out couple of problems related to conditional requirements and add some more tests.

@xedin xedin mentioned this pull request May 15, 2019
Merged
xedin added 3 commits May 15, 2019 14:21
@xedin
[ConstraintSystem] Switch "missing conformance" fix/diagnostic to use…
a23bc3e 
… protocol type

Switch type allows "missing conformance" diagnostic to be
used not only for protocols by for protocol compositions as well.
@xedin
[ConstraintSystem] Use missing conformance fix to diagnose contextu…
577e629 
…al failures

Extend use of `missing protocol conformance` fix to cover contextual
failures, such as:

- Assignment mismatches, where destination requires source to conform
  to certain protocol (or protocol composition);
- Incorrect returns where returned type doesn't conform to the
  protocol specified in the signature.
@xedin
[ConstraintSystem] Move missing contextual protocol detection to `mat…
d5c561b 
…chExistentialTypes`
@xedin xedin force-pushed the diag-missing-contextual-conformances branch from 6cc11db to d5c561b Compare May 15, 2019 22:37
@xedin xedin marked this pull request as ready for review May 15, 2019 22:38
@xedin
Copy link
Contributor Author

xedin commented May 15, 2019

@slavapestov I have moved fix to matchExistentialTypes, which seems to be a much better place for it, and allows to diagnose failures related to protocol compositions more precisely by pointing out exactly which conformances are missing. Please take a look.

@xedin
Copy link
Contributor Author

xedin commented May 16, 2019

@swift-ci please test

@swift-ci
Copy link
Contributor

Build failed
Swift Test OS X Platform
Git Sha - 6cc11dbf0ff3d9032caf6c29c1e904d1ed28db38

@swift-ci
Copy link
Contributor

Build failed
Swift Test Linux Platform
Git Sha - 6cc11dbf0ff3d9032caf6c29c1e904d1ed28db38

DougGregor
DougGregor approved these changes May 16, 2019
View reviewed changes
lib/Sema/CSDiag.cpp
contextualType->is<ProtocolCompositionType>())
diagID = diagIDProtocol;

if (contextualType->isExistentialType()) {
Copy link
Member

Choose a reason for hiding this comment

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

Should we do the same for opaque result types and/or AnyHashable? They have the same kinds of conformance-related issues.

Copy link
Contributor Author

@xedin xedin May 16, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

Yeah, those would already be diagnosed by new diagnostic framework, I have a couple of examples added to the suite in my previous PR :)

@xedin xedin merged commit 5fb3a8b into swiftlang:master May 17, 2019
@davezarzycki
Copy link
Contributor

This broke one test on my Linux machine (Red Hat Fedora 30). Was this expected?

: 'RUN: at line 1';   /home/dave/s/u/t/bin/swift -frontend -target x86_64-unknown-linux-gnu  -module-cache-path '/home/dave/s/u/t/swift-test-results/x86_64-unknown-linux-gnu/clang-module-cache' -swift-version 4   -typecheck -verify -disable-objc-attr-requires-foundation-module /home/dave/s/u/swift/test/stmt/foreach.swift
--
Exit Code: 1

Command Output (stderr):
--
/home/dave/s/u/swift/test/stmt/foreach.swift:64:58: error: incorrect message found
  for f : ElementProtocol in gir { } // expected-error {{sequence element type 'Int' does not conform to expected type 'ElementProtocol'}}
                                                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
                                                         type 'Int' does not conform to protocol 'ElementProtocol'

@xedin
Copy link
Contributor Author

xedin commented May 17, 2019

Yeah, I should have re-ran the tests...

@xedin xedin mentioned this pull request May 17, 2019
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@slavapestov slavapestov slavapestov left review comments

@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.
5 participants
@xedin @swift-ci @davezarzycki @slavapestov @DougGregor