RFR(S): 8007772: G1: assert(!hr->isHumongous() || mr.start() == hr->bottom()) failed: the start of HeapRegion and MemRegion should be consistent for humongous regions

[Jesper Wilhelmsson]

OK, so you don't think a comment would avoid miss interpretation?
I'm fine either way really. I just try to avoid code duplication.
Ship it!
/Jesper


11 feb 2013 kl. 23:35 skrev John Cuthbertson <john.cuthbertson at oracle.com>:

> Hi Jesper,
> 
> On 2/11/2013 12:51 PM, Jesper Wilhelmsson wrote:
>> I guess it's a matter of taste, to me it is a lot clearer. But sure, it takes a second look at the code to see that short-circuiting matters here.
>> 
>> The version with code duplication is a bit confusing to me and requires a second look to see that there is no difference between the different clauses in the if/else if.
>> 
>> I would argue that for someone who is new to the code, the non-duplicated code would be easier to get familiar with due to (at a first glance) less complicated code. And you don't have to know that the short-circuiting is necessary until you know the code well enough to realize that it is necessary, if you see what I mean.
>> 
>> We could also add a comment :-)
>> /Jesper
>> 
> 
> We could argue but I don't think that you will convince me that your suggestion is clearer. I first read the code the same way as Bengt and was going to make a similar comment as his until an "Aha!" moment and I saw the short-circuit. Reading the code incorrectly has biased me. :)
> 
> Thanks,
> 
> JohnC