Skip to content
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

Address the exceptionally high build time. #495

Merged
merged 2 commits into from
Aug 11, 2017
Merged

Address the exceptionally high build time. #495

merged 2 commits into from
Aug 11, 2017

Conversation

andersio
Copy link
Member

@andersio andersio commented Jul 23, 2017

Swift 3 compiler is less performant in situations that need sophisticated inference. It appears not to be an issue for Swift 4 though. πŸ’β€β™‚οΈ

While the constraints have been tightened, IMO it shouldn't be considered API breaking, since the Property requirements mandate NoError, and the Error associated type is just an implementation detail that is always NoError at conformance time.

Ideally we would get rid of these in RAS 3.0 by constraining PropertyProtocol with Error == NoError.

Checklist

  • Updated CHANGELOG.md.

@andersio andersio added the bug label Jul 23, 2017
@andersio andersio changed the title Addressed the exceptionally high build time. Address the exceptionally high build time. Jul 23, 2017
@andersio andersio force-pushed the build-time branch 3 times, most recently from 817699d to 1afaabb Compare July 23, 2017 15:18
@mdiep
Copy link
Contributor

mdiep commented Jul 25, 2017

Changes to type constraints always make me nervous because of the potential to change the type inference. πŸ™ˆ

@mdiep
Copy link
Contributor

mdiep commented Aug 9, 2017

Would adding associatedtype Error = NoError directly on PropertyProtocol have the same effect?

If not, I'm πŸ‘ on this.

@andersio
Copy link
Member Author

andersio commented Aug 9, 2017

master and default type for Error:
screen shot 2017-08-09 at 11 14 12 pm

This branch:
screen shot 2017-08-09 at 11 16 03 pm

It seems somehow the problem isn't severe in master now. But I did manage to blow it up to minutes in #444 when dealing with some compiler nonsenses. πŸ™ˆ

@andersio andersio merged commit 4e23c21 into master Aug 11, 2017
@andersio andersio deleted the build-time branch August 11, 2017 07:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants