RFR: JDK-8301074: Replace NULL with nullptr in share/opto/

[Vladimir Kozlov]

On Wed, 25 Jan 2023 11:46:36 GMT, Johan Sjölen <jsjolen at openjdk.org> wrote:

> Hi, this PR changes all occurrences of NULL to nullptr for the subdirectory share/opto/. Unfortunately the script that does the change isn't perfect, and so we
> need to comb through these manually to make sure nothing has gone wrong. I also review these changes but things slip past my eyes sometimes.
> 
> Here are some typical things to look out for:
> 
>     No changes but copyright header changed (probably because I reverted some changes but forgot the copyright).
>     Macros having their NULL changed to nullptr, these are added to the script when I find them. They should be NULL.
>     nullptr in comments and logs. We try to use lower case "null" in these cases as it reads better. An exception is made when code expressions are in a comment.
> 
> An example of this:
> 
> 
> // This function returns null
> void* ret_null();
> // This function returns true if *x == nullptr
> bool is_nullptr(void** x);
> 
> 
> Note how nullptr participates in a code expression here, we really are talking about the specific value nullptr.
> 
> Thanks!

In general this looks good. I found few cases need to change.

src/hotspot/share/opto/compile.hpp line 219:

> 217:   };
> 218: 
> 219:   // Variant of TraceTime(null, &_t_accumulator, CITime);

nullptr

src/hotspot/share/opto/connode.hpp line 81:

> 79:       return new ConPNode( TypePtr::NULL_PTR ) ;
> 80:     else
> 81:       return new ConPNode( TypeRawPtr::make(con) );

Please also add `{}` for this ` if else` since you are changing this code. Code style.

src/hotspot/share/opto/convertnode.cpp line 52:

> 50:   if( t == TypePtr::NULL_PTR ) return TypeInt::ZERO;
> 51:   const TypePtr *tp = t->isa_ptr();
> 52:   if( tp != nullptr ) {

Please remove spaces: `if (tp != nullptr) {`

src/hotspot/share/opto/doCall.cpp line 904:

> 902: void Parse::catch_inline_exceptions(SafePointNode* ex_map) {
> 903:   // Caller is responsible for saving away the map for normal control flow!
> 904:   assert(stopped(), "call set_map(null) first");

nullptr

src/hotspot/share/opto/escape.cpp line 98:

> 96:   ResourceMark rm;
> 97: 
> 98:   // Add ConP#null and ConN#null nodes before ConnectionGraph construction

`ConP and ConN null oop nodes`

src/hotspot/share/opto/graphKit.cpp line 522:

> 520:       // Cf. case Bytecodes::_athrow in parse2.cpp.
> 521:       uncommon_trap(reason, Deoptimization::Action_none,
> 522:                     (ciKlass*)nullptr, (char*)nullptr, must_throw);

Do we need these casts? I am still not sure when casts should be used with `nullptr`.

src/hotspot/share/opto/graphKit.cpp line 653:

> 651:   // of in-edges on the call to the uncommon trap.
> 652: 
> 653:   uncommon_trap(reason, action, (ciKlass*)nullptr, (char*)nullptr, must_throw);

Casts?

src/hotspot/share/opto/graphKit.cpp line 3064:

> 3062:   }
> 3063: 
> 3064:   // type == null if profiling tells us this object is always null

`type is null`

src/hotspot/share/opto/graphKit.cpp line 3517:

> 3515: // If the given klass is a constant or known to be an array,
> 3516: // fetch the constant layout helper value into constant_value
> 3517: // and return (Node*)null.  Otherwise, load the non-constant

`and return null.`

src/hotspot/share/opto/library_call.cpp line 5142:

> 5140:   // if alloc == null we don't have to worry about a tightly coupled allocation so we can emit all needed guards
> 5141:   // if saved_jvms_before_guards != nullptr (then alloc != nullptr) then we can handle guards and a tightly coupled allocation
> 5142:   // if saved_jvms_before_guards == nullptr and alloc != nullptr, we can't emit any guards

Replace `!= nullptr` with `is not null` and `== nullptr` with `is null`.

src/hotspot/share/opto/library_call.cpp line 5164:

> 5162: 
> 5163:   if (!can_emit_guards) {
> 5164:     // if saved_jvms_before_guards == nullptr and alloc != nullptr, we don't emit any

`is null` and `is not null`

src/hotspot/share/opto/machnode.hpp line 349:

> 347:   // Returns the MachOper as determined by memory_operand(), for use, if
> 348:   // needed by the caller. If (MachOper *)-1 is returned, base and index
> 349:   // are set to NodeSentinel. If (MachOper *) null is returned, base and

`If null is returned`

src/hotspot/share/opto/phaseX.cpp line 2223:

> 2221:     for( uint i = 0; i < cnt; i++ ) {          // For all inputs do
> 2222:       Node *input = clone->in(i);
> 2223:       if( input != nullptr ) {                    // Ignore nulls

comment's alignment

src/hotspot/share/opto/subnode.cpp line 1777:

> 1775:   //    // to be incremented in a private block on a loop backedge.
> 1776:   //    if( du && du->cnt(this) && du->out(this)[0]->Opcode() == Op_CountedLoopEnd )
> 1777:   //      return null;

nullptr

src/hotspot/share/opto/subnode.cpp line 1782:

> 1780:   //    // Gets triggered by too many simple optimizations to be bothered with
> 1781:   //    // re-trying it again and again.
> 1782:   //    if( !phase->allow_progress() ) return null;

nullptr

src/hotspot/share/opto/superword.cpp line 4652:

> 4650:   if(_slp->is_trace_alignment()) {
> 4651:     print_depth(); tty->print_cr(" %d SWPointer::offset_plus_k: FAILED since another invariant has been detected before", n->_idx);
> 4652:     print_depth(); tty->print("  \\ %d SWPointer::offset_plus_k: _invar != nullptr: ", _invar->_idx); _invar->dump();

`is not null`

src/hotspot/share/opto/type.cpp line 2687:

> 2685:   const TypePtr* res_ptr = res->is_ptr();
> 2686:   if (res_ptr->speculative() != nullptr) {
> 2687:     // type->speculative() == null means that speculation is no better

`is null`

-------------

PR: https://git.openjdk.org/jdk/pull/12187