rmuir commented on PR #11923:
URL: https://github.com/apache/lucene/pull/11923#issuecomment-1312221930
This NarrowCalculation is specific to multiplication, which was the issue
for #11905. It would not have detected the multiplication issue for the bug
before that (#11861), as that one never involved `long` at all, just a pure
integer overflow in integer space.
Instead here, the idea is that its always suspicious if you see `long z = x
* y`, where x and y are integers. A good example is simpletext code doing stuff
like `seek(docid * 8)` which is bad, it will overflow at 270M docs. so you
should do `seek(docid * 8L)` instead.
The "IntLongMath" is more aggressive and looks at not just multiplication
but other operations too. example of other stuff it picks up:
```
lucene/core/src/java/org/apache/lucene/search/DocIdSetIterator.java:131:
warning: [IntLongMath] Expression of type int may overflow before being
assigned to a long
return maxDoc - minDoc;
^
(see https://errorprone.info/bugpattern/IntLongMath)
Did you mean 'return (long) maxDoc - minDoc;'?
```
Hence I'm not sure about enabling `IntLongMath`, I think it might be more of
a hassle than it is worth. I feel like this `NarrowCalculation` is a decent
tradeoff. Multiplication was responsible for the last 2 overflow bugs in
vectors. Normally when multiplying the developer starts thinks about overflow
already, so the suggestions that it makes are not unnatural.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
