Skip to content

[GSB] AbstractProtocol requirements are never explicit. #9856

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
May 24, 2017

Conversation

huonw
Copy link
Contributor

@huonw huonw commented May 23, 2017

If they're considered explicit, requirement inference will complain when
it infers X: SomeProtocol for some concrete type X.

Fixes SR-4693, rdar://problem/31819616

I suspect this isn't correct, but the git history didn't reveal the reasoning, I'm not sure I fully grasped the space, and, tests seemed to pass locally.

@huonw huonw requested a review from DougGregor May 23, 2017 01:44
@huonw
Copy link
Contributor Author

huonw commented May 23, 2017

@swift-ci Please smoke test

@huonw
Copy link
Contributor Author

huonw commented May 23, 2017

@swift-ci Please test

Copy link
Member

@DougGregor DougGregor left a comment

Choose a reason for hiding this comment

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

I think the intent here was to allow us to detect redundant requirements within a protocol definition itself. Maybe we should be checking the inferred bit on protocolReq instead of always returning false ?

@huonw
Copy link
Contributor Author

huonw commented May 23, 2017

Aha, that makes sense.

@huonw huonw force-pushed the concrete-types-are-concrete branch from 6fec176 to 6f3bb1e Compare May 23, 2017 23:15
@huonw
Copy link
Contributor Author

huonw commented May 23, 2017

@swift-ci Please smoke test

@huonw
Copy link
Contributor Author

huonw commented May 23, 2017

@swift-ci Please smoke test.

2 similar comments
@huonw
Copy link
Contributor Author

huonw commented May 24, 2017

@swift-ci Please smoke test.

@huonw
Copy link
Contributor Author

huonw commented May 24, 2017

@swift-ci Please smoke test.

@huonw
Copy link
Contributor Author

huonw commented May 24, 2017

This is a legitimate timeout. The change now causes the compiler to do the infinite recursion/expansion thing in the standard library.

@huonw huonw force-pushed the concrete-types-are-concrete branch from 6f3bb1e to 9ecf87e Compare May 24, 2017 20:27
…and not inferred.

If they're considered explicit, requirement inference will complain when
it infers X: SomeProtocol for some concrete type X.

Fixes SR-4693, rdar://problem/31819616
@huonw huonw force-pushed the concrete-types-are-concrete branch from 9ecf87e to 578097c Compare May 24, 2017 20:32
@huonw
Copy link
Contributor Author

huonw commented May 24, 2017

@swift-ci Please smoke test and merge.

@huonw
Copy link
Contributor Author

huonw commented May 24, 2017

@swift-ci Please smoke test

@huonw huonw merged commit 9228c04 into swiftlang:master May 24, 2017
@huonw huonw deleted the concrete-types-are-concrete branch May 24, 2017 21:51
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.

2 participants