RFR (XS): 80357290: Code using assert(is_oop_or_null) needs better error messages

[Thomas Schatzl]

Hi Marcus,

On Wed, 2014-08-06 at 15:42 +0200, Marcus Larsson wrote:
> Hi Thomas,
> 
> I like your suggestion and updated the changeset.
> 
> New CR:
> http://cr.openjdk.java.net/~brutisso/webrev-8035729v2/
> 

  looks okay to me. Maybe for completeness the text "Expected an oop for
<type> field at " should be changed to "Expected an oop or NULL
for ...". I forgot that in my listing, sorry.

But it's probably already a big step up from "Not an oop?" :)  If you
want to change these minor issues anyway, consider it reviewed as well.

Thanks,
  Thomas


> On 08/06/2014 11:24 AM, Thomas Schatzl wrote:
> > Hi,
> >
> > On Wed, 2014-08-06 at 10:28 +0200, Marcus Larsson wrote:
> >> Hi,
> >>
> >> Can I have reviews for this small change adding oop information to
> >> is_oop_or_null assert fail messages?
> >>
> >> CR:
> >> http://cr.openjdk.java.net/~brutisso/webrev-8035729/
> >>
> >> Bug:
> >> https://bugs.openjdk.java.net/browse/JDK-8035729/
> >    not sure what the other think, but is it worth to try to make the
> > error messages themselves be more uniform?
> > While I do not want to be all the same, we have:
> >
> > 1) expected an oop or NULL
> > 2) expected an oop or NULL:
> > 3) Not an oop? (
> > 4) check for header:
> > 5) should be an oop now:
> > 6) Object should be whole at this point:
> > 7) Not an oop:
> > 8) discovered field is bad:
> > 9) bad referent:
> > 10) bad next field:
> > 11) bad discovered field:
> > 12) should always be an oop:
> >
> > Some suggestions:
> >
> > - Capitalize the first letter of the sentences
> > - change 1 to "Expected an oop or NULL at "
> > - replace 2,3,5,6,7,12 by 1
> > - change 4,9-11 to "Expected an oop for <type> field at "
> >
> > Otherwise the change is good.
> >
> > Thanks,
> >    Thomas
> >
> >
> >
>