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

[Jon Masamitsu]

On 08/07/2014 01:08 AM, Marcus Larsson wrote:
> Hi Jon,
>
> Putting the assert in a function would remove the information about 
> file and line number for triggered asserts.

Yes, you're right about that but the information
about where the assertion failed is in the hs_err
log so should almost always be available.

> Perhaps we could make a macro for this, but I think it might just be 
> simpler to keep them as regular asserts in this case.

Not worth a macro.  Your call.  I think it makes it
easier to maintain.  I personally would trade the
line number/file info in the assert message for
the reduced code duplication.

Jon

>
> Thanks,
> Marcus
>
>
> On 08/06/2014 07:43 PM, Jon Masamitsu wrote:
>> Marcus,
>>
>> There are debug functions that just do assertion checking.
>> For example, assert_locked_or_safepoint() in mutexLocker.cpp.
>> Would it make it easier to create an
>>
>> void assert_is_oop_or_null(oop obj) {
>>   assert(obj->is_oop_or_null(), err_msg())"Expected an oop or NULL at 
>> " PTR_FORMAT, p2i(obj);
>> }
>>
>> and use that where you can?  Some of the uses have more specific 
>> messages
>> so this won't work everywhere.
>>
>> Jon
>>
>>
>>
>> On 8/6/2014 6:42 AM, Marcus Larsson wrote:
>>> Hi Thomas,
>>>
>>> I like your suggestion and updated the changeset.
>>>
>>> New CR:
>>> http://cr.openjdk.java.net/~brutisso/webrev-8035729v2/
>>>
>>> Thanks,
>>> Marcus
>>>
>>> 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
>>>>
>>>>
>>>>
>>>
>>
>