RFR: 8064811: Use THREAD instead of CHECK_NULL in return statements

[Stefan Karlsson]

On 2014-11-13 16:33, Coleen Phillimore wrote:
>
> The thing that I worry about with this change is that if someone adds 
> code later after the return, they'll miss changing the THREAD 
> parameter back into CHECK.


I was thinking the same, but I knew the we used this idiom in other 
places in the JVM and thought that changing these would be OK. An 
alternative approach would be to always read out the value into a variable:

  Klass* ConstantPool::klass_ref_at(int which, TRAPS) {
   Klass* k = klass_at(klass_ref_index_at(which), CHECK_NULL);
   return k;
  }

I can do that if people feel more comfortable with it.

Thanks,
Stefank

>   But maybe it's okay because the thing returned will be NULL and code 
> is likely to crash on someone trying to use the value returned. Ok.  
> This is a good cleanup.  I'm surprised there weren't more.
>
> Thanks,
> Coleen
>
> On 11/13/14, 9:59 AM, Stefan Karlsson wrote:
>> Hi all,
>>
>> Please, review this patch to replace usages of the CHECK_ macros in 
>> return statements, with the THREAD define.
>>
>> http://cr.openjdk.java.net/~stefank/8064811/webrev.01/
>> https://bugs.openjdk.java.net/browse/JDK-8064811
>>
>> From the bug report:
>>
>> Take the following method as an example:
>>  Klass* ConstantPool::klass_ref_at(int which, TRAPS) {
>>   return klass_at(klass_ref_index_at(which), CHECK_NULL);
>>  }
>>
>> This will expand into:
>>  Klass* ConstantPool::klass_ref_at(int which, TRAPS) {
>>   return klass_at(klass_ref_index_at(which), THREAD);
>>   if (HAS_PENDING_EXCEPTIONS) {
>>     return NULL;
>>   }
>>   (void)(0);
>>  }
>>
>> The if-statement will never be reached.
>>
>> We have seen cases where the compiler warns about this, and the 
>> recent change to enable -Wreturn-type will make this more likely to 
>> happen.
>>
>> The suggested solution is to change the example above into:
>>  Klass* ConstantPool::klass_ref_at(int which, TRAPS) {
>>   return klass_at(klass_ref_index_at(which), THREAD);
>>  }
>>
>> thanks,
>> StefanK
>