remove code below asserts in limitedmap.h (fixes a ToDo) #4782

pull Diapolo wants to merge 1 commits into bitcoin:master from Diapolo:fix_assert_todo changing 1 files +6 −9

Diapolo commented at 9:04 PM on August 28, 2014: none

No description provided.
in src/limitedmap.h:None in 8cf5ee0a94 outdated
```
  59 | @@ -61,12 +60,11 @@ template <typename K, typename V> class limitedmap
  60 |                  return;
  61 |              }
  62 |          // Shouldn't ever get here
  63 | -        assert(0); //TODO remove me
```
laanwj commented at 2:54 AM on August 30, 2014:

Why was this added as a TODO in the first place? It's not that removing a line is so much work that it needs to be postponed. This makes me think there is some more to be done here before it can be safely removed.

laanwj commented at 9:24 AM on August 30, 2014:

I see that limitedmap.h was added by @thebluematt, with TODO and all. Any specific reason that this was added as a TODO?

TheBlueMatt commented at 6:31 AM on September 2, 2014:

I believe the thought was "I'll leave this here until we waited a while and never hit this, then we can be reasonably sure its actually as dead code as I think it is".
TheBlueMatt commented at 6:32 AM on September 2, 2014: member

ACK on the last line and the one comment change, conceptual ACK on either removing the comments that say "//TODO" or the whole line, whichever people think is a better idea.
Diapolo commented at 8:15 AM on September 9, 2014: none

@TheBlueMatt Removing what exactly will make this an ACK from you, didn't get it, sorry.
laanwj commented at 9:24 AM on September 10, 2014: member

If the code in question is supposed to be never reached (and thus it is undefined behaviour to get there), keeping an assert there is the proper thing to do.
Diapolo commented at 9:37 AM on September 10, 2014: none

@laanwj So keep the asserts and remove the "dead code" below them and the Todo comment?
laanwj commented at 8:06 AM on September 18, 2014: member

Yes, sounds good to me.
Diapolo renamed this:
~~remove asserts in limitedmap.h (fixes a ToDo)~~
remove code below asserts in limitedmap.h (fixes a ToDo)
on Sep 18, 2014
Diapolo commented at 12:17 PM on September 18, 2014: none

@laanwj Done...
remove code below asserts in limitedmap.h (fixes a ToDo) 2e5361b9c2
Diapolo commented at 7:36 AM on September 22, 2014: none

@laanwj Rebased after the formatting changes.
BitcoinPullTester commented at 7:50 AM on September 22, 2014: none

Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/p4782_2e5361b9c20517a22a4d1fdae3077d23800ecba7/ for binaries and test log. This test script verifies pulls every time they are updated. It, however, dies sometimes and fails to test properly. If you are waiting on a test, please check timestamps to verify that the test.log is moving at http://jenkins.bluematt.me/pull-tester/current/ Contact BlueMatt on freenode if something looks broken.
laanwj merged this on Sep 25, 2014
laanwj closed this on Sep 25, 2014
laanwj referenced this in commit 82e370b4b0 on Sep 25, 2014
Diapolo deleted the branch on Sep 25, 2014
MarcoFalke locked this on Sep 8, 2021

Contributors