add {.push raises: [].} to more files
#5943
Conversation
Extend `{.push raises: [].}` to all files with spec references that are
periodically updated.
- #5942
| @@ -5,6 +5,8 @@ | |||
| # * Apache v2 license (license terms in the root directory or at https://www.apache.org/licenses/LICENSE-2.0). | |||
| # at your option. This file may not be copied, modified, or distributed except according to those terms. | |||
|
|
|||
| {.push raises: [].} | |||
There was a problem hiding this comment.
This didn't work for me in this file -- I reverted the automated changes, since the point of the automated PR in fixtures_utils.nim is to be risk-free and to literally nothing interesting semantically. Adding some {.push raises: [].} is okay, but changing the content to support them isn't, for that PR.
Also, the linter error message shows {.push raises: []}. I'm not sure if that works too, but it's not consistent with what the codebase is using.
There was a problem hiding this comment.
Yeah, extracted the addition of the {.push raises: [].} to here so that the other PR can remain as a purely automated change.
| @@ -5,6 +5,7 @@ | |||
| # * Apache v2 license (license terms in the root directory or at https://www.apache.org/licenses/LICENSE-2.0). | |||
| # at your option. This file may not be copied, modified, or distributed except according to those terms. | |||
|
|
|||
| {.push raises: [].} | |||
There was a problem hiding this comment.
nimbus-eth2/tests/consensus_spec/test_fixture_kzg.nim(39, 48) Error: relativePath(path, suitePath, '/') can raise an unlisted exception: Exception
also reverted this from the automated PR for similar reasons -- easy or hard to fix, doesn't matter. It's a semantic change and that PR should have exactly zero semantic changes.
There was a problem hiding this comment.
That particular exception seems strange to me, but I spent no time investigating it at all yet. If nothing else, relativePath wasn't/isn't core functionality and there are definitely other ways of achieving the same thing; it was just slightly more convenient at the time.
There was a problem hiding this comment.
relativePath raises when the arguments are not referring to paths. Had the same in fork choice test suite and extracted it to a reusable helper.
There was a problem hiding this comment.
Also
{.push raises: [].}
raise newException(ValueError, "")compiles and, predictably, raises.
I guess {.push raises: [].} only works in procs/funcs or maybe templates or iterators. My current working model is, maybe it needs something to attach to.
There was a problem hiding this comment.
yeah, notably, it also fails for {.async.}. it's a best-effort thing, not exhaustive for sure.
There was a problem hiding this comment.
Well, async is a custom pragma implemented by a non-stdlib thing in Chronos's case which just transforms the incoming function arbitrarily. Hard to pin that on Nim necessarily. This isn't.
|
#5942 has merged and covers of the more trivial parts of this -- at least, could ensure that it's not going to insert duplicate |
Extend
{.push raises: [].}to all files with spec references that are periodically updated.