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

[Lindenmaier, Goetz]

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.

Best regards,
  Goetz 

-----Original Message-----
From: Kim Barrett [mailto:kim.barrett at oracle.com] 
Sent: Dienstag, 3. März 2015 03:02
To: Lindenmaier, Goetz
Cc: hotspot-dev Source Developers
Subject: Re: RFR(M): 8073315: Enable gcc -Wtype-limits and fix upcoming issues.

On Mar 2, 2015, at 4:48 PM, Lindenmaier, Goetz <goetz.lindenmaier at sap.com> wrote:
> 
> I will look into heapRetionSet.cpp tomorrow.

OK.  If it turns out to be hard to prove the tests should be > rather than >= (e.g. proving that
if !is_empty() then those values must *not* be zero), then I'd be ok with just having the test
simplified, and maybe file a CR to investigate further.

> About the comment, I don't feel good documenting something
> that's not true, as the other implementations still return 0 on 
> error.

OK.  I'll adjust the corresponding comment in the CR I filed.

> I can't see where that file is coming from, I sure never did a 
> hg add for it.  It used to be there. An artefact of pull & rebase?  
> I'll remove it again...

Can't help you there; just make sure it's gone when pushing.