On Tue, 8 Sep 2020 20:30:36 GMT, Éamonn McManus 
<github.com+5246810+eamonnmcma...@openjdk.org> wrote:

>> This is a follow-up of the Mercurial-based workflow initiated on the 
>> core-lib-devs mailing list [0]. Not sure if this
>> one is strictly necessary or if the patches sent on the list are sufficient. 
>> Anyway, I exploit this PR as a test ;-)
>> [0] 
>> https://mail.openjdk.java.net/pipermail/core-libs-dev/2020-September/068474.html
>
> src/java.base/share/classes/java/lang/Long.java line 1697:
> 
>> 1695:             final long q = (dividend >>> 1) / divisor << 1;
>> 1696:             final long r = dividend - q * divisor;
>> 1697:             return r - (~(r - divisor) >> Long.SIZE - 1 & divisor);
> 
> Parentheses would be particularly helpful here. I'd certainly have to think 
> hard about the relative precedences of
> `>>`, `-`, and `&`, whereas I wouldn't have to with: return r - ((~(r - 
> divisor) >> (Long.SIZE - 1)) & divisor);
> 
> I also think it would be worth adding a comment saying that this is 
> deliberately `>>` not `>>>`, even though we have
> `>>>` in the divide method. Here we're propagating the sign bit so that 
> `thing & divisor` is either 0 or `divisor`
> according as `thing` is –1 or 0.

Hi Éamonn,

I have no problems adding non-strictly needed parentheses. I usually don't 
because I'm quite familiar with operator
precedence and often forget that other people are not.

I'll add comments for parts, like the one you point out, that are not already 
discussed at length in the
bibliographical reference.

Thanks
Raffaello

-------------

PR: https://git.openjdk.java.net/jdk/pull/31

Reply via email to