`system/inclrtl` has unexpected side-effect for exception tracking

[markspanbroek]

system/inclrtl pushes sinkInference, which can lead to unexpected results when you're working with exception tracking using {.push raises:[...].} statements.

This confusion is the root cause of https://github.com/status-im/nim-chronos/issues/169.

Example

{.push raises: [Defect].}

include "system/inclrtl"

{.pop.}

proc foo* =
  raise newException(CatchableError, "boo!")

{.push raises: [Defect].}

The {.pop.} in the example does not pop the raises pragma as you might expect, but the sinkInference pragma instead.

Current Output

/home/user/repro.nim(7, 1) template/generic instantiation from here
/home/user/repro.nim(8, 3) Error: can raise an unlisted exception: ref CatchableError

Expected Output

No errors

Additional Information

$ nim -v
Nim Compiler Version 1.4.4 [Linux: amd64]
Compiled at 2021-03-25
Copyright (c) 2006-2020 by Andreas Rumpf

git hash: 2ff517462bf8609b30e6134c96658aa7912b628a
active boot switches: -d:release

[ghost]

Wait, but why would you want to include this file in your user code?

[ghost]

And the underlying problem with closures with sink inference is known and that's why sink inference is disabled for user code

[ghost]

oh, in your case it causes issues with exception tracking because of the pop, interesting

but yeah, anyway, I don't see how inclrtl is used in the asyncloop.nim file

[markspanbroek]

The file was included in chronos because it started out as a copy of async from the standard library. Now that we've found the root cause, we're indeed likely to remove the include.

[ghost]

@markspanbroek yeah, just remove that line - chronos doesn't use it at all, it's only needed for making RTL procs. I tested and chronos compiles just fine with that line removed

[arnetheduck]

Notably this is a regression from nim 1.2 where there is no push in inclrtl - the include itself is a leftover from past times that we'll remove but we're opening the issue to highlight the regression itself and a general problem with push inside files that are designed for including.

[ghost]

@arnetheduck sure, but then this issue should have a more generic title about push pragma in include'd files :)