Use "+ 1" instead of "++" for the assertion
Diff Detail
- Repository
- rL LLVM
- Build Status
Buildable 17872 Build 17872: arc lint + arc unit
Event Timeline
I don't know this code well enough to LGTM this, but can you talk a bit about why, please? This side-effect-inside-an-assert looks sane to me.
It actually looks wrong to me to *not* have the side effect. If this is due
to some warning, then the increment needs to be moved after the assert
bah. I misread the code originally. Its less about a warning and more about an oddity when reading code. that said, I completely mis-read this and agreed, the loop index should be incremented outside of the assert.
Having the side-effect inside the assert is harmless since the #ifndef ensures the whole loop is only built when asserts are enabled and Index is only used by the assert. However, I agree it's weird to have side-effects inside the assert. Moving the increment out of the assert and into the for loop would LGTM
The whole loop is an assert. The only purpose of it is to verify the ID vs. its position in the loop. Maybe having for (unsigned i = ...; i != e; ++i) instead would be cleaner, but I think it's ok as is. I don't have a problem with changing it either, but I wouldn't say it's necessary.