RFR(M): 8073315: Enable gcc -Wtype-limits and fix upcoming issues.

[Kim Barrett]

On Mar 3, 2015, at 9:46 AM, Lindenmaier, Goetz <goetz.lindenmaier at sap.com> wrote:
> 
> Hi Kim,
> 
> as I understand the code, > 0 should hold for both tests.
> But I think what is really meant by the check, protecting against underflow, 
> should be checked in decrement():
> 
> --- //bas2/sapjvm/dev/6/hotspot/src/share/vm/gc_implementation/g1/heapRegionSet.hpp     2015-03-03 09:00:44.000000000 0100
> +++ /sapmnt/home/d045726/src3/sapjvm/dev/6/hotspot/src/share/vm/gc_implementation/g1/heapRegionSet.hpp  2015-03-03 09:00:44.000000000 0100
> @@ -67,6 +67,9 @@
>   }
> 
>   void decrement(const uint length_to_remove, const size_t capacity_to_remove) {
> +#ifdef HEAP_REGION_SET_FORCE_VERIFY
> +    guarantee(length_to_remove <= _length && capacity_to_remove <= _capacity, "underflow");
> +#endif
>     _length -= length_to_remove;
>     _capacity -= capacity_to_remove;
>   }
> 
> Or by casting _length and _capacity to singed types in the guarantee in verify(), which is 
> quite ugly.  But I think it would work fine by the values one can expect here. 
> 
> I changed verify() to check for '>', but didn't add above guarantee, I think that again
> is out of the scope of this change.
> http://cr.openjdk.java.net/~goetz/webrevs/8073315-wUnsign/webrev.05/
> 
> I ran a row of tests with the adapted assertion, and will push it into our nightbuild.

I think you are right that the check is really for underflow, and that should be in decrement()
as you suggest.  And I’m fine with the way you’ve left it.  Anything further can be a later cleanup
if desired.

So I’m done reviewing.  Looks good to me.