Skip to content

[GSB] Diagnose explicit constraints made redundant by inferred ones. #8281

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

Conversation

DougGregor
Copy link
Member

@DougGregor DougGregor commented Mar 22, 2017

Extend the diagnosis of redundant constraints to also include explicitly-specified constraints made redundant by inferred constraints. The canonical example is something like:

 func f<T: Hashable>(_: Set<T>)

where the parameter of type Set<T> implies T: Hashable. With this PR, we start warning about the explicit T: Hashable constraint being redundant.

@DougGregor
Copy link
Member Author

@dabrahams take a look at the second commit

@DougGregor DougGregor force-pushed the redundant-explicit-via-inferred branch from bb0082a to 2e28311 Compare March 23, 2017 00:35
@DougGregor DougGregor changed the title [WIP Do Not Merge] [GSB] Diagnose explicit constraints made redundant by inferred ones. [GSB] Diagnose explicit constraints made redundant by inferred ones. Mar 23, 2017
@DougGregor
Copy link
Member Author

@phausler I'd like your opinion on the Foundation commit: is the resulting code clearer or not?

@gribozavr You argued against this a while ago, with a crappy prototype of this warning. Now that I've gone through the exercise of doing the warning right and updating the standard library, I think I like it, but didn't want to put it in without giving you a chance to pipe up.

@DougGregor
Copy link
Member Author

@swift-ci please smoke test

@DougGregor
Copy link
Member Author

I'm going to go ahead and merge based on positive feedback from @dabrahams and my own experience working with this warning. We can back it out if we decide we don't like it.

@DougGregor DougGregor merged commit 607732f into swiftlang:master Mar 23, 2017
@DougGregor DougGregor deleted the redundant-explicit-via-inferred branch March 23, 2017 03:50
@phausler
Copy link
Contributor

The change to measurement makes sense to me, no Measurement can ever be anything but Unit. Furthermore the CoreData change looks good to me as well.

@jrose-apple
Copy link
Contributor

Very sad about this change. :-( This is definitely the opposite direction from what I'd want.

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.

3 participants