On Mon, 3 Nov 2025 18:09:01 GMT, Raffaello Giulietti <[email protected]> wrote:
>> test/langtools/tools/javac/lint/ShiftOutOfRange.java line 14: >> >>> 12: >>> 13: // These should generate warnings >>> 14: a = a << (byte)-1; >> >> Looking at the patch, I am not completely convinced disallowing `-1` is >> beneficial. On one hand, it looks weird. On the other hand, it makes some >> sense (as far as I can tell, it is universal for both `int` and `long`). >> And, unlike `128`, it is not clearly wrong. I am not sure if it wouldn't be >> better to simply permit `-1` as a special case. >> >> I see there are `<< -5` in the microbenchmark tests, but I think I am less >> inclined to accommodate negative values other than `-1`, as the semantics of >> those is much more cryptic, for me at least. >> >> Please let me know what you think. > > One way to read `v << -n` is: "Shift out everything to the left, except for > the `n` least significant bits". > Analogously for `v >>> -n`: "Shift out everything to the right, except for > the `n` most significant bits". > > To rotate `v` by `n` bits to the left, for example, one can write `v << n | v > >>> -n` (regardless of the width of `v`) Hmm, obviously this is a judgement call and I'm curious what others think. My opinion is that I still think there should be a warning for shifts of -1. I mean, maybe to a _real_ hacker, then of course `foo << -1` makes perfect sense because it's just stating the "obvious" which is "shift the low order bit into the high order position and set all the other bits to zero", right?? :) I just don't think the average Java programmer automatically understands that. Yes, it's an idiom or "handy trick", but if hasn't attained the status of being universally recognized then it doesn't deserve a special exception. On the other hand, excluding -1 from the warning would be less disruptive (at least, to the real hackers out there), and that has its own merits. So I don't have a super strong opinion about it. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/27102#discussion_r2487476314
