fix noreturn/implicit discard check logic (#23681)

fixes #10440, fixes #13871, fixes #14665, fixes #19672, fixes #23677

The false positive in #23677 was caused by behavior in
`implicitlyDiscardable` where only the last node of `if`/`case`/`try`
etc expressions were considered, as in the final node of the final
branch (in this case `else`). To fix this we use the same iteration in
`implicitlyDiscardable` that we use in `endsInNoReturn`, with the
difference that for an `if`/`case`/`try` statement to be implicitly
discardable, all of its branches must be implicitly discardable.
`noreturn` calls are also considered implicitly discardable for this
reason, otherwise stuff like `if true: discardableCall() else: error()`
doesn't compile.

However `endsInNoReturn` also had bugs, one where `finally` was
considered in noreturn checking when it shouldn't, another where only
`nkIfStmt` was checked and not `nkIfExpr`, and the node given for the
error message was bad. So `endsInNoReturn` now skips over
`skipForDiscardable` which no longer contains
`nkIfStmt`/`nkCaseStmt`/`nkTryStmt`, stores the first encountered
returning node in a var parameter for the error message, and handles
`finally` and `nkIfExpr`.

Fixing #23677 already broke a line in `syncio` so some package code
might be affected.

(cherry picked from commit 42e8472ca6)
This commit is contained in:
metagn
2024-06-05 21:53:05 +03:00
committed by narimiran
parent 7821aa94eb
commit d0a8637872
6 changed files with 195 additions and 74 deletions

View File

@@ -874,7 +874,7 @@ proc writeFile*(filename: string, content: openArray[byte]) {.since: (1, 1).} =
var f: File = nil
if open(f, filename, fmWrite):
try:
f.writeBuffer(unsafeAddr content[0], content.len)
discard f.writeBuffer(unsafeAddr content[0], content.len)
finally:
close(f)
else: