search

status-im / nimbus-eth2

Nim implementation of the Ethereum Beacon Chain
https://nimbus.guide
Other
523 stars 225 forks source link

[Tech debt] Clean up warnings and XDeclaredButNotUsed #880

Closed tersec closed 4 years ago

tersec commented 4 years ago

At least at present, the UnusedImport warnings are not reliable enough to act on. The remaining ones are:

nim-beacon-chain/beacon_chain/beacon_node.nim(1040, 3) Warning: Use setTimer/clearTimer instead; addTimer is deprecated [Deprecated]
nim-beacon-chain/beacon_chain/beacon_node.nim(885, 9) Warning: Use setTimer/clearTimer instead; addTimer is deprecated [Deprecated]
nim-beacon-chain/beacon_chain/beacon_node.nim(892, 3) Warning: Use setTimer/clearTimer instead; addTimer is deprecated [Deprecated]
nim-beacon-chain/beacon_chain/eth2_network.nim(244, 24) Warning: Use one[T](varargs[Future[T]]); or is deprecated [Deprecated]
nim-beacon-chain/beacon_chain/eth2_network.nim(274, 32) Warning: Use one[T](varargs[Future[T]]); or is deprecated [Deprecated]
nim-beacon-chain/beacon_chain/eth2_network.nim(314, 20) Warning: Use one[T](varargs[Future[T]]); or is deprecated [Deprecated]
nim-beacon-chain/beacon_chain/eth2_network.nim(387, 19) Warning: Use one[T](varargs[Future[T]]); or is deprecated [Deprecated]
nim-beacon-chain/beacon_chain/eth2_network.nim(437, 19) Warning: Use one[T](varargs[Future[T]]); or is deprecated [Deprecated]
nim-beacon-chain/beacon_chain/eth2_network.nim(483, 9) Warning: Use allFutures(varargs[Future[T]]); all is deprecated [Deprecated]
nim-beacon-chain/beacon_chain/time.nim(82, 3) Warning: Use setTimer/clearTimer instead; addTimer is deprecated [Deprecated]

This could form part of https://github.com/status-im/nim-beacon-chain/issues/867

zah commented 4 years ago

Have we filed a Nim issue for this?

tersec commented 4 years ago

Have we filed a Nim issue for this?

No, and it looks like it was mostly an artifact, not a Nim issue: https://github.com/status-im/nim-beacon-chain/pull/881

tersec commented 4 years ago

All distinct remaining warnings with https://github.com/status-im/nim-beacon-chain/pull/884 applied:

beacon_chain/eth2_network.nim(16, 26) Warning: imported and not used: 'messages' [UnusedImport]
beacon_chain/eth2_network.nim(244, 24) Warning: Use one[T](varargs[Future[T]]); or is deprecated [Deprecated]
beacon_chain/eth2_network.nim(274, 32) Warning: Use one[T](varargs[Future[T]]); or is deprecated [Deprecated]
beacon_chain/eth2_network.nim(314, 20) Warning: Use one[T](varargs[Future[T]]); or is deprecated [Deprecated]
beacon_chain/eth2_network.nim(387, 19) Warning: Use one[T](varargs[Future[T]]); or is deprecated [Deprecated]
beacon_chain/eth2_network.nim(437, 19) Warning: Use one[T](varargs[Future[T]]); or is deprecated [Deprecated]
beacon_chain/eth2_network.nim(483, 9) Warning: Use allFutures(varargs[Future[T]]); all is deprecated [Deprecated]
tests/fork_choice/interpreter.nim(12, 7) Warning: import results; result is deprecated [Deprecated]
tests/fork_choice/interpreter.nim(17, 1) Warning: result is deprecated [Deprecated]
tersec commented 4 years ago

All distinct warnings through https://github.com/status-im/nim-beacon-chain/pull/914:

beacon_chain/eth2_network.nim(16, 26) Warning: imported and not used: 'messages' [UnusedImport]
beacon_chain/eth2_network.nim(505, 9) Warning: Use allFutures(varargs[Future[T]]); all is deprecated [Deprecated]
tests/test_block_pool.nim(11, 13) Warning: imported and not used: 'chronicles' [UnusedImport]
tersec commented 4 years ago

Probably few enough warnings not to be useful to track through this issue anymore, and they're not hidden during builds, so closing.

The test_block_pool was/is a way of allowing one to nim c block_pool outside the rest of the tests.

tersec commented 4 years ago

Lots of warnings again.

tersec commented 4 years ago

be475a82d74dc8b579aa12ae8a2afdbb5b8caf05 (current devel) when building beacon_node and test make targets results in the following unique warnings, by reverse-sorted-counts:

     10 vendor/nim-serialization/serialization.nim(45, 20) Warning: Cannot prove that 'result' is initialized. This will become a compile time error in the future. [ProveInit]
     10 beacon_chain/beacon_chain_db.nim(84, 3) Warning: Cannot prove that 'res' is initialized. This will become a compile time error in the future. [ProveInit]
      3 vendor/nim-chronos/chronos/asyncmacro2.nim(222, 37) Warning: Cannot prove that 'result' is initialized. This will become a compile time error in the future. [ProveInit]
      2 tests/test_mainchain_monitor.nim(42, 8) Warning: method has lock level <unknown>, but another method has 0 [LockLevel]
      2 tests/test_mainchain_monitor.nim(35, 8) Warning: method has lock level <unknown>, but another method has 0 [LockLevel]
      2 tests/test_mainchain_monitor.nim(26, 8) Warning: method has lock level <unknown>, but another method has 0 [LockLevel]
      2 tests/test_mainchain_monitor.nim(19, 8) Warning: use {.base.} for base methods; baseless methods are deprecated [UseBase]
      2 tests/test_mainchain_monitor.nim(12, 8) Warning: method has lock level <unknown>, but another method has 0 [LockLevel]
      2 tests/test_block_pool.nim(11, 13) Warning: imported and not used: 'chronicles' [UnusedImport]
      2 tests/official/fixtures_utils.nim(42, 3) Warning: Cannot prove that 'result' is initialized. This will become a compile time error in the future. [ProveInit]
tersec commented 4 years ago

As of https://github.com/status-im/nim-beacon-chain/commit/28d6cd25242d2f55e9becd191f1d7914a2028d9f

      3 vendor/nim-serialization/serialization.nim(45, 20) Warning: Cannot prove that 'result' is initialized. This will become a compile time error in the future. [ProveInit]
      3 vendor/nim-chronos/chronos/asyncmacro2.nim(222, 37) Warning: Cannot prove that 'result' is initialized. This will become a compile time error in the future. [ProveInit]
      2 tests/test_mainchain_monitor.nim(42, 8) Warning: method has lock level <unknown>, but another method has 0 [LockLevel]
      2 tests/test_mainchain_monitor.nim(35, 8) Warning: method has lock level <unknown>, but another method has 0 [LockLevel]
      2 tests/test_mainchain_monitor.nim(26, 8) Warning: method has lock level <unknown>, but another method has 0 [LockLevel]
      2 tests/test_mainchain_monitor.nim(19, 8) Warning: use {.base.} for base methods; baseless methods are deprecated [UseBase]
      2 tests/test_mainchain_monitor.nim(12, 8) Warning: method has lock level <unknown>, but another method has 0 [LockLevel]
      2 tests/test_block_pool.nim(11, 13) Warning: imported and not used: 'chronicles' [UnusedImport]
      2 tests/official/fixtures_utils.nim(42, 3) Warning: Cannot prove that 'result' is initialized. This will become a compile time error in the future. [ProveInit]
tersec commented 4 years ago

As of https://github.com/status-im/nim-beacon-chain/commit/2ddc507e5b21ff3485fb78b62e43a75abf4ab051:

grep -E "(XDeclaredButNotUsed|Warning:)" build.txt | grep -Ev "(Defect|SszSizeMismatchError|IOError|vendor/)" | sed -e"s/^.home.user.nim-beacon-chain.//" | sort | uniq -c | sort -n
      1 beacon_chain/beacon_node.nim(484, 9) Hint: 'left' is declared but not used [XDeclaredButNotUsed]
      1 beacon_chain/beacon_node.nim(590, 7) Hint: 'beaconTime' is declared but not used [XDeclaredButNotUsed]
      1 beacon_chain/beacon_node.nim(817, 8) Hint: 'processPromptCommands' is declared but not used [XDeclaredButNotUsed]
      1 beacon_chain/request_manager.nim(18, 6) Hint: 'fetchAncestorBlocksFromPeer' is declared but not used [XDeclaredButNotUsed]
      1 beacon_chain/sync_protocol.nim(60, 6) Hint: 'importBlocks' is declared but not used [XDeclaredButNotUsed]
      1 tests/official/test_fixture_ssz_generic_types.nim(57, 3) Hint: 'ComplexTestStruct' is declared but not used [XDeclaredButNotUsed]
      1 tests/official/test_fixture_ssz_generic_types.nim(66, 3) Hint: 'BitsStruct' is declared but not used [XDeclaredButNotUsed]
      1  [XDeclaredButNotUsed]
      2 beacon_chain/fork_choice/fork_choice.nim(410, 8) Hint: 'tMove_out_of_tree' is declared but not used [XDeclaredButNotUsed]
      2 tests/test_block_pool.nim(11, 13) Warning: imported and not used: 'chronicles' [UnusedImport]
      2 tests/test_mainchain_monitor.nim(12, 8) Warning: method has lock level <unknown>, but another method has 0 [LockLevel]
      2 tests/test_mainchain_monitor.nim(19, 8) Warning: use {.base.} for base methods; baseless methods are deprecated [UseBase]
      2 tests/test_mainchain_monitor.nim(26, 8) Warning: method has lock level <unknown>, but another method has 0 [LockLevel]
      2 tests/test_mainchain_monitor.nim(35, 8) Warning: method has lock level <unknown>, but another method has 0 [LockLevel]
      2 tests/test_mainchain_monitor.nim(42, 8) Warning: method has lock level <unknown>, but another method has 0 [LockLevel]
      3 beacon_chain/eth2_network.nim(176, 3) Hint: 'readTimeoutErrorMsg' is declared but not used [XDeclaredButNotUsed]
      3 beacon_chain/eth2_network.nim(236, 6) Hint: 'peerId' is declared but not used [XDeclaredButNotUsed]
      3 beacon_chain/eth2_network.nim(957, 6) Hint: 'toPeerInfo' is declared but not used [XDeclaredButNotUsed]
      3 beacon_chain/eth2_network.nim(966, 8) Hint: 'checkIfConnectedToBootstrapNode' is declared but not used [XDeclaredButNotUsed]
      4 beacon_chain/block_pool.nim(229, 5) Hint: 'justifiedBlock' is declared but not used [XDeclaredButNotUsed]
      5 beacon_chain/eth2_network.nim(482, 9) Hint: 'msgName' is declared but not used [XDeclaredButNotUsed]
     12 beacon_chain/block_pool.nim(353, 37) Hint: 'state' is declared but not used [XDeclaredButNotUsed]
     12 beacon_chain/block_pool.nim(865, 6) Hint: 'delState' is declared but not used [XDeclaredButNotUsed]
     14 beacon_chain/spec/state_transition_epoch.nim(59, 14) Hint: 'epoch_transition_justification_and_finalization' is declared but not used [XDeclaredButNotUsed]
     14 beacon_chain/spec/state_transition_epoch.nim(60, 14) Hint: 'epoch_transition_times_rewards_and_penalties' is declared but not used [XDeclaredButNotUsed]
     14 beacon_chain/spec/state_transition_epoch.nim(61, 14) Hint: 'epoch_transition_registry_updates' is declared but not used [XDeclaredButNotUsed]
     14 beacon_chain/spec/state_transition_epoch.nim(62, 14) Hint: 'epoch_transition_slashings' is declared but not used [XDeclaredButNotUsed]
     14 beacon_chain/spec/state_transition_epoch.nim(63, 14) Hint: 'epoch_transition_final_updates' is declared but not used [XDeclaredButNotUsed]
     14 nbench/bench_lab.nim(58, 10) Hint: 'ntag' is declared but not used [XDeclaredButNotUsed]
     15 beacon_chain/ssz.nim(36, 3) Hint: 'maxChunkTreeDepth' is declared but not used [XDeclaredButNotUsed]
tersec commented 4 years ago

as of https://github.com/status-im/nim-beacon-chain/commit/74db0f3c8d76d59f95d10f81fde0855f02b67048

      1 beacon_chain/beacon_node.nim(484, 9) Hint: 'left' is declared but not used [XDeclaredButNotUsed]
      1 beacon_chain/beacon_node.nim(590, 7) Hint: 'beaconTime' is declared but not used [XDeclaredButNotUsed]
      1 beacon_chain/beacon_node.nim(817, 8) Hint: 'processPromptCommands' is declared but not used [XDeclaredButNotUsed]
      1 beacon_chain/request_manager.nim(18, 6) Hint: 'fetchAncestorBlocksFromPeer' is declared but not used [XDeclaredButNotUsed]
      1 beacon_chain/sync_protocol.nim(60, 6) Hint: 'importBlocks' is declared but not used [XDeclaredButNotUsed]
      1 nbench/reports.nim(14, 10) Hint: 'cpuX86' is declared but not used [XDeclaredButNotUsed]
      1 nbench/reports.nim(47, 11) Hint: 'avgCyclesBillions' is declared but not used [XDeclaredButNotUsed]
      1 tests/official/test_fixture_ssz_generic_types.nim(57, 3) Hint: 'ComplexTestStruct' is declared but not used [XDeclaredButNotUsed]
      1 tests/official/test_fixture_ssz_generic_types.nim(66, 3) Hint: 'BitsStruct' is declared but not used [XDeclaredButNotUsed]
      1 tests/test_block_pool.nim(13, 23) Warning: imported and not used: 'helpers' [UnusedImport]
      2 beacon_chain/fork_choice/fork_choice.nim(410, 8) Hint: 'tMove_out_of_tree' is declared but not used [XDeclaredButNotUsed]
      2 tests/test_block_pool.nim(11, 3) Warning: imported and not used: 'chronicles' [UnusedImport]
      2 tests/test_mainchain_monitor.nim(12, 8) Warning: method has lock level <unknown>, but another method has 0 [LockLevel]
      2 tests/test_mainchain_monitor.nim(19, 8) Warning: use {.base.} for base methods; baseless methods are deprecated [UseBase]
      2 tests/test_mainchain_monitor.nim(26, 8) Warning: method has lock level <unknown>, but another method has 0 [LockLevel]
      2 tests/test_mainchain_monitor.nim(35, 8) Warning: method has lock level <unknown>, but another method has 0 [LockLevel]
      2 tests/test_mainchain_monitor.nim(42, 8) Warning: method has lock level <unknown>, but another method has 0 [LockLevel]
      3 beacon_chain/eth2_network.nim(176, 3) Hint: 'readTimeoutErrorMsg' is declared but not used [XDeclaredButNotUsed]
      3 beacon_chain/eth2_network.nim(236, 6) Hint: 'peerId' is declared but not used [XDeclaredButNotUsed]
      3 beacon_chain/eth2_network.nim(957, 6) Hint: 'toPeerInfo' is declared but not used [XDeclaredButNotUsed]
      3 beacon_chain/eth2_network.nim(966, 8) Hint: 'checkIfConnectedToBootstrapNode' is declared but not used [XDeclaredButNotUsed]
     12 beacon_chain/block_pool.nim(352, 37) Hint: 'state' is declared but not used [XDeclaredButNotUsed]
     12 beacon_chain/block_pool.nim(864, 6) Hint: 'delState' is declared but not used [XDeclaredButNotUsed]
     15 beacon_chain/spec/state_transition_epoch.nim(59, 14) Hint: 'epoch_transition_justification_and_finalization' is declared but not used [XDeclaredButNotUsed]
     15 beacon_chain/spec/state_transition_epoch.nim(60, 14) Hint: 'epoch_transition_times_rewards_and_penalties' is declared but not used [XDeclaredButNotUsed]
     15 beacon_chain/spec/state_transition_epoch.nim(61, 14) Hint: 'epoch_transition_registry_updates' is declared but not used [XDeclaredButNotUsed]
     15 beacon_chain/spec/state_transition_epoch.nim(62, 14) Hint: 'epoch_transition_slashings' is declared but not used [XDeclaredButNotUsed]
     15 beacon_chain/spec/state_transition_epoch.nim(63, 14) Hint: 'epoch_transition_final_updates' is declared but not used [XDeclaredButNotUsed]
     16 nbench/bench_lab.nim(59, 10) Hint: 'ntag' is declared but not used [XDeclaredButNotUsed]
tersec commented 4 years ago

This is probably not worth keeping open as a tracking bug.