feat: add panic builtin function
#757
Conversation
ss2165
requested review from
qartik and
mark-koch
and removed request for
qartik
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #757 +/- ##
==========================================
- Coverage 93.00% 92.97% -0.04%
==========================================
Files 71 71
Lines 8178 8222 +44
==========================================
+ Hits 7606 7644 +38
- Misses 572 578 +6 ☔ View full report in Codecov by Sentry. |
|
|
||
| vals = [ExprSynthesizer(self.ctx).synthesize(val)[0] for val in rest] | ||
| node = PanicExpr(msg.value, vals) | ||
| return with_loc(self.node, node), NoneType() |
There was a problem hiding this comment.
Should panic be generic over the type it returns? This would require also implementing check to infer the return type.
Imo the proper way to implement this would be to use a bottom return type like Python's Never or NoReturn for panic, but that's too much for this PR
There was a problem hiding this comment.
I couldn't think of a good reason for panic returning anything other than None, do you think it is important?
There was a problem hiding this comment.
For example, it's currently not possible to make the type checker understand that panics are an early exit. So, say you want to implement Option.unwrap yourself, then you still need to explicitly return something in the error case:
def unwrap(opt: Option[T]) -> T:
match opt:
case Some(x): return x
case Nothing: return panic("Is None")
Co-authored-by: Mark Koch <[email protected]>
|
@mark-koch thanks for the fix! Feel free to approve if you are happy with the rest |
Closes #756