The presence of the dummy element is already checked with
the if ((int)stack.size() < i) { check near the beginning of this
opcode.
Details
Details
- Reviewers
deadalnix - Group Reviewers
Restricted Project
make check
Diff Detail
Diff Detail
- Repository
- rABC Bitcoin ABC
- Branch
- rms3
- Lint
Lint Passed - Unit
No Test Coverage - Build Status
Buildable 6788 Build 11623: Bitcoin ABC Buildbot (legacy) Build 11622: arc lint + arc unit
Event Timeline
Comment Actions
This makes me a bit worried, to be honest. Isn't it possible to move the nulldummy check near the bound check? This will avoid the spooky action at a distance effect (which is probably why this redundant check survived here in the first place).
Comment Actions
Yes, I intend to do this in a follow-up diff, however that's technically a semantic change and the redundant check has to be removed one way or another. I'm not sure what spooky interaction you're alluding to 🤔 .
Comment Actions
This code must remain "obviously correct" as much as possible. Systematically doing bound check before accessing an element is absolutely part of that process.
If you move the access, then it become obvious that the previous bound check covers it. Right now it is spooky action at a distance.