jameysharp opened PR #5392 from isle-partial-flag
to main
:
I speculate that this PR may have a performance impact, which I want to measure before merging, but I'd appreciate review on the implementation. Also I wanted to get the CI run going overnight.
Instead of tying fallibility of constructors to whether they're either internal or pure, this commit assumes all constructors are infallible unless tagged otherwise with a "partial" flag.
Internal constructors without the "partial" flag are not allowed to use constructors which have the "partial" flag on the right-hand side of any rules, because they have no way to report last-minute match failures.
Multi-constructors should never be "partial"; they report match failures with an empty iterator instead. In turn this means you can't use partial constructors on the right-hand side of internal multi-constructor rules. However, you can use the same constructors on the left-hand side with
if
orif-let
instead.In many cases, ISLE can already trivially prove that an internal constructor always returns
Some
. With this commit, those cases are laregly unchanged, except for removing all theOption
s andSome
s from the generated code for those terms.However, for internal non-partial constructors where ISLE could not prove that, it now emits an
unreachable!
panic as the last-resort, instead of returningNone
like it used to do. Among the existing backends, here's how many constructors have these panic cases:
- x64: 14% (53/374)
- aarch64: 15% (41/277)
- riscv64: 23% (26/114)
- s390x: 47% (268/567)
It's often possible to rewrite rules so that ISLE can tell the panic can never be hit. Just ensure that there's a lowest-priority rule which has no constraints on the left-hand side.
But in many of these constructors, it's difficult to statically prove the unhandled cases are unreachable because that's only down to knowledge about how they're called or other preconditions.
So this commit does not try to enforce that all terms have a last-resort fallback rule.
jameysharp requested cfallin for a review on PR #5392.
jameysharp updated PR #5392 from isle-partial-flag
to main
.
jameysharp updated PR #5392 from isle-partial-flag
to main
.
jameysharp updated PR #5392 from isle-partial-flag
to main
.
jameysharp has marked PR #5392 as ready for review.
cfallin submitted PR review.
jameysharp merged PR #5392.
Last updated: Nov 22 2024 at 17:03 UTC