Implement language feature #6885 #6954
Conversation
|
I see failed tests, I have an idea to fix it |
| result = x | ||
| if b.kind in {tyExpr, tyNil, tyVoid}: result = x | ||
| elif a.kind in {tyExpr, tyNil}: result = y | ||
| if a.kind in {tyExpr, tyNil}: result = y |
There was a problem hiding this comment.
in the latest version, they are no longer swapped. In first version, I was using tyVoid tag on raise statement and needed to check b for tyVoid before a. In latest version, raise statement has no type as originally, I am checking for node kinds instead, a and b checks are reverted to the original order
compiler/sem.nim
Outdated
|
|
||
| proc commonType*(x: PType, y: PNode): PType = | ||
| # ignore exception raising branches in case/if expressions | ||
| if y.kind == nkRaiseStmt: return x |
There was a problem hiding this comment.
This is very good, but not completely correct. You need to do something like
proc endsInRaise(n: PNode): bool =
var it = n
while it.kind in {nkStmtList, nkStmtListExpr}: it = it.lastSon
result = it.kind == nkRaiseStmt
|
@cooldome closed them |
|
Issues shouldn't be closed until the PRs get merged. This hasn't been merged yet. |
|
Oh, I thought this was merged |
compiler/sem.nim
Outdated
| proc endsInNoReturn(n: PNode): bool = | ||
| # check if expr ends in raise exception or call of noreturn proc | ||
| var it = n | ||
| while it.kind in {nkStmtList, nkStmtListExpr}: |
compiler/sem.nim
Outdated
| while it.kind in {nkStmtList, nkStmtListExpr}: | ||
| it = it.lastSon | ||
| result = it.kind in nkCallKinds and sfNoReturn in it[0].sym.flags | ||
| result = result or it.kind == nkRaiseStmt |
There was a problem hiding this comment.
Merge these two result assignments into one.
|
One more attempt, more cases covered |
|
Just to let you know I have one more pull request that is going to use procSymInCallExpr(): disallow statements after noreturn proc call, just like statements after break, continue and return are disallowed |
|
One more thing. endsInNoReturn is called twice for every branch in if/case expressions. I can make it only a single call per branch and store intermediate result in seq[bool] and reuse second time, but I am not sure if it is going to make any difference. Let me know if you think, it is worth it optimization |
compiler/sem.nim
Outdated
| proc procSymInCallExpr(n: PNode): PSym = | ||
| assert(n.kind in nkCallKinds) | ||
| var it = n | ||
| while it.kind != nkSym: |
There was a problem hiding this comment.
That is not correct and not necessary! Should produce a crashing loop for f()(). Proc calls are not static in Nim, they can always go through a procvar!
There was a problem hiding this comment.
Thanks for clarification, now it is clear that dotExprs that I was seeing were calls via function pointers and should be ignored.
procSymInCallExpr is removed.
compiler/sem.nim
Outdated
| while it.kind in {nkStmtList, nkStmtListExpr} and it.len > 0: | ||
| it = it.lastSon | ||
| result = it.kind == nkRaiseStmt or | ||
| (it.kind in nkCallKinds and sfNoReturn in procSymInCallExpr(it).flags) |
There was a problem hiding this comment.
The correct check is it.kind in nkCallKinds and it[0].kind == nkSym and sfNoReturn in it[0].sym.flags
|
Add more sematics checks for noreturns procs: return type is not allowed for noreturn procs, statements after call to noreturn procs are no longer allowed. |
| noVal(it) | ||
| incl(sym.flags, sfNoReturn) | ||
| if sym.ast[paramsPos][0].kind != nkEmpty: | ||
| localError(sym.ast[paramsPos][0].info, errNoReturnWithReturnTypeNotAllowed) |
There was a problem hiding this comment.
I'll merge this now anyway, but this check is not 100% correct, void as a return type would be valid.
The idea is simple: ignore always raising exceptions branches in if/case exprs, so return deducted type can be not nil. The following now compiles:
Type of a variable is bool, exception raising branch is ignored. I don't expect any backwards in compatible behavior, as previously such code didn't compile.
Implementation detail: raise stmt tyVoid tag was selected at random, to be able to detect it in commonType() proc. It could be another type tag, let me if there are better alternatives.
Tests added to cover cases that should not compile and several other that should not compile