search

nim-lang / Nim

Nim is a statically typed compiled systems programming language. It combines successful concepts from mature languages like Python, Ada and Modula. Its design focuses on efficiency, expressiveness, and elegance (in that order of priority).
https://nim-lang.org
Other
16.22k stars 1.46k forks source link

fix noreturn/implicit discard check logic #23681

Closed metagn closed 3 weeks ago

metagn commented 3 weeks ago

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.

Araq commented 3 weeks ago

Superb work.

github-actions[bot] commented 3 weeks ago

Thanks for your hard work on this PR! The lines below are statistics of the Nim compiler built from 42e8472ca6eab740c0879428bd119ec94e70fe74

Hint: mm: orc; opt: speed; options: -d:release 178683 lines; 8.401s; 753.098MiB peakmem