Skip to content
This repository has been archived by the owner on Jul 3, 2022. It is now read-only.

Fix compiler crash in Release mode take 2 #53

Closed
wants to merge 1 commit into from
Closed

Fix compiler crash in Release mode take 2 #53

wants to merge 1 commit into from

Conversation

sync
Copy link
Contributor

@sync sync commented May 28, 2015

if convert NoError to a class the compiler stop crashing in Release. (if it's an enum / struct it crashes)

@Thomvis
Copy link
Owner

Thomvis commented May 28, 2015

Thanks for the PR and thanks for discovering the crash. Unfortunately, I don't think that the proposed change is an acceptable solution.

The great thing about NoError being an enum without any cases is that you simply cannot create it, giving a strong guarantee that futures/results typed with NoError cannot fail. Making it a class undermines that principle.

The crash has to be solved, of course, and it would help if you could create an issue describing the crash (including the stacktrace of the compiler crash, if possible) and if you could test if the crash still happens if you set the optimization to -Onone.

If you do have a different solution to this issue, please update the PR accordingly or feel free to close this if we should discuss it in a separate issue first.

@sync
Copy link
Contributor Author

sync commented May 29, 2015

I understand and really liked the fact that it was an enum. You could instead do this (again I do understand this is a temporary solution why someone sort out why in Release and when building for a iOS Device it is crashing the compiler, this branch got updated with this fix):

public class NoError {
    private init() {
        fatalError("impossible to instantiate NoError")
    }
}

Thomvis added a commit that referenced this pull request May 30, 2015
@Thomvis
Copy link
Owner

Thomvis commented May 30, 2015

I've managed to find a durable fix for this problem. It turned out to have more to do with BrightFuturesError than with NoError. This fix solves two segmentation faults in the tests, but it would be interesting if you could see if it also resolves the crashes in the cases where you ran into it.

@sync
Copy link
Contributor Author

sync commented May 31, 2015

👍 fixed thanks

@sync sync closed this May 31, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants