Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

add {.push raises: [].} to more files #5943

Merged
merged 8 commits into from
Feb 22, 2024
Merged

add {.push raises: [].} to more files #5943

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
2dccf85
add `{.push raises: [].}` to more files
etan-status Feb 21, 2024
a740ae9
fix hint in CI error message
etan-status Feb 21, 2024
02a2657
extract `relativeTestPathComponent` to `fixtures_utils` and fix raises
etan-status Feb 21, 2024
ef36072
fix `test_fixture_kzg` raises
etan-status Feb 21, 2024
de9d557
add `{.raises.}` to `walkTests`
etan-status Feb 21, 2024
305ec63
`parseTest` for JSON
etan-status Feb 21, 2024
cdefb4c
Merge branch 'unstable' into dev/etan/nm-raisesspec
etan-status Feb 21, 2024
ec65d2f
avoid duplicate push
etan-status Feb 21, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions beacon_chain/rpc/rest_config_api.nim
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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: [].}

import stew/[byteutils, base10], chronicles
import ".."/beacon_node,
".."/spec/forks,
Expand Down
2 changes: 2 additions & 0 deletions beacon_chain/spec/datatypes/constants.nim
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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: [].}

import chronos/timer

type
Expand Down
2 changes: 2 additions & 0 deletions beacon_chain/spec/presets/mainnet/altair_preset.nim
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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: [].}

# Mainnet preset - Altair
# https://github.com/ethereum/consensus-specs/blob/v1.4.0-beta.6/presets/mainnet/altair.yaml
const
Expand Down
2 changes: 2 additions & 0 deletions beacon_chain/spec/presets/mainnet/bellatrix_preset.nim
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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: [].}

# Mainnet preset - Bellatrix
# https://github.com/ethereum/consensus-specs/blob/v1.4.0-beta.6/presets/mainnet/bellatrix.yaml
const
Expand Down
2 changes: 2 additions & 0 deletions beacon_chain/spec/presets/mainnet/capella_preset.nim
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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: [].}

# Mainnet preset - Capella
# https://github.com/ethereum/consensus-specs/blob/v1.4.0-beta.6/presets/mainnet/capella.yaml
const
Expand Down
2 changes: 2 additions & 0 deletions beacon_chain/spec/presets/mainnet/deneb_preset.nim
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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: [].}

# Mainnet preset - Deneb
# https://github.com/ethereum/consensus-specs/blob/v1.4.0-beta.6/presets/mainnet/deneb.yaml
const
Expand Down
2 changes: 2 additions & 0 deletions beacon_chain/spec/presets/minimal/altair_preset.nim
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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: [].}

# Minimal preset - Altair
# https://github.com/ethereum/consensus-specs/blob/v1.4.0-beta.6/presets/minimal/altair.yaml
const
Expand Down
2 changes: 2 additions & 0 deletions beacon_chain/spec/presets/minimal/bellatrix_preset.nim
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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: [].}

# Minimal preset - Bellatrix
# https://github.com/ethereum/consensus-specs/blob/v1.4.0-beta.6/presets/minimal/bellatrix.yaml
const
Expand Down
2 changes: 2 additions & 0 deletions beacon_chain/spec/presets/minimal/capella_preset.nim
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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: [].}

# Minimal preset - Capella
# https://github.com/ethereum/consensus-specs/blob/v1.4.0-beta.6/presets/minimal/capella.yaml
const
Expand Down
2 changes: 2 additions & 0 deletions beacon_chain/spec/presets/minimal/deneb_preset.nim
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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: [].}

# Minimal preset - Deneb
# https://github.com/ethereum/consensus-specs/blob/v1.4.0-beta.6/presets/minimal/deneb.yaml
const
Expand Down
1 change: 1 addition & 0 deletions tests/consensus_spec/altair/test_fixture_light_client_sync_protocol.nim
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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: [].}
{.used.}

import
Expand Down
2 changes: 2 additions & 0 deletions tests/consensus_spec/fixtures_utils.nim
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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: [].}
Copy link
Contributor

@tersec tersec Feb 21, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, extracted the addition of the {.push raises: [].} to here so that the other PR can remain as a purely automated change.


import
# Standard library
std/[strutils, typetraits],
Expand Down
1 change: 1 addition & 0 deletions tests/consensus_spec/test_fixture_kzg.nim
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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: [].}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, notably, it also fails for {.async.}. it's a best-effort thing, not exhaustive for sure.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

{.used.}

import
Expand Down
2 changes: 2 additions & 0 deletions tests/testblockutil.nim
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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: [].}

import
chronicles,
stew/endians2,
Expand Down
Loading