Skip to content
This repository was archived by the owner on Oct 31, 2025. It is now read-only.

Intercept panic calls and replace them with aborts.#305

Merged
mergify[bot] merged 6 commits intoEmbarkStudios:mainfrom
LykenSol:panic-at-the-spirsco
Dec 3, 2020
Merged

Intercept panic calls and replace them with aborts.#305
mergify[bot] merged 6 commits intoEmbarkStudios:mainfrom
LykenSol:panic-at-the-spirsco

Conversation

@eddyb
Copy link
Copy Markdown
Contributor

@eddyb eddyb commented Dec 2, 2020

Where "abort" currently is equivalent to loop {}, but we could try to do something better in the future.

(Ideally we'd use a future variant of OpKill that works in all shaders, and ideally has the right semantics of leaving some kind of indication that there was an error in some shader invocation, but that might be asking for too much)


The reason this is "WIP", even if it does work (see the test changes), is because, in order to defer errors from creating unsupported constants (mostly &panic::Location) passed to panic entry-points, I had to turn them into zombies.
So the tests only work because the panic calls are replaced with "abort"s, and the zombie constants never used.

From my discussion with @khyperia, we should be able to use zombies here (and in other places), if we can properly track Spans for them, and I believe that's doable, but haven't gotten around to it.

EDIT: that's all done, thanks to #311!

@eddyb eddyb requested a review from khyperia December 2, 2020 22:09
@eddyb eddyb changed the title [WIP] Intercept panic calls and replace them with aborts. Intercept panic calls and replace them with aborts. Dec 3, 2020
@eddyb eddyb marked this pull request as ready for review December 3, 2020 17:42
@mergify mergify bot merged commit 6c7ca97 into EmbarkStudios:main Dec 3, 2020
@eddyb eddyb deleted the panic-at-the-spirsco branch December 4, 2020 05:02
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