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

Mon Feb 20 12:56:03 UTC 2023

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!

Okay, so most of this stuff looks good. At some points there are conventions that I'm unfamiliar with, I've tried my best to comment those.

I have now applied the manual review fixes which I found.

src/hotspot/share/opto/block.cpp line 1236:

> 1234:           }
> 1235:         }
> 1236:         // If anything has been inserted (n2 != null), continue after last node inserted.

nullptr

src/hotspot/share/opto/buildOopMap.cpp line 508:

> 506:             } else if (base != fp || offset == Type::OffsetBot) {
> 507:               // Do nothing: the fp operand is either not from a memory use
> 508:               // (base == null) OR the fp is used in a non-memory context

nullptr

src/hotspot/share/opto/c2_globals.hpp line 388:

> 386:           "IP address to connect to visualizer")                            \
> 387:                                                                             \
> 388:   notproduct(ccstr, PrintIdealGraphFile, nullptr,                              \

align

src/hotspot/share/opto/callGenerator.cpp line 1181:

> 1179: JVMState* PredicatedIntrinsicGenerator::generate(JVMState* jvms) {
> 1180:   // The code we want to generate here is:
> 1181:   //    if (receiver == null)

nullptr

src/hotspot/share/opto/callnode.cpp line 317:

> 315:     if (p->_method == nullptr)          return true;   // bci is irrelevant
> 316:     if (p->_bci    != q->_bci)       return false;
> 317:     if (p->_reexecute != q->_reexecute)  return false;

align

src/hotspot/share/opto/cfgnode.cpp line 1672:

> 1670:     case BoolTest::ge: cmp_zero_idx = 2; phi_x_idx = true_path;  break;
> 1671:     default:           return nullptr;                              break;
> 1672:     }

align

src/hotspot/share/opto/chaitin.cpp line 2459:

> 2457:                   } else {
> 2458:                     assert(check->bottom_type()->is_ptr()->_offset == 0, "Bad base pointer");
> 2459:                     // Base either ConP(null) or loadConP

Be explicit about `nullptr` here, we don't want it to look like Java null.

src/hotspot/share/opto/compile.cpp line 2126:

> 2124:   // "inlining_incrementally() == false" is used to signal that no inlining is allowed
> 2125:   // (see LateInlineVirtualCallGenerator::do_late_inline_check() for details).
> 2126:   // Tracking and verification of modified nodes is disabled by setting "_modified_nodes == null"

nullptr

src/hotspot/share/opto/compile.cpp line 3455:

> 3453:           //
> 3454:           // and the uncommon path (== null) will use narrow_oop_reg directly
> 3455:           // since narrow oops can be used in debug info now (see the code in

Use nullptr here to be very explicit which null is being talked about.

src/hotspot/share/opto/domgraph.cpp line 183:

> 181:       t->_bucket = nullptr;
> 182:       if (pre_order == 1)
> 183:         t->_parent = nullptr;          // first block doesn't have parent

align

src/hotspot/share/opto/domgraph.cpp line 529:

> 527:       w->_label = w;                   // DFS to vertex map
> 528:       w->_ancestor = nullptr;             // Fast LINK & EVAL setup
> 529:       w->_child = &ntarjan[0];         // Sentinel

align

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

> 57:   add_java_object(C->top(), PointsToNode::GlobalEscape);
> 58:   phantom_obj = ptnode_adr(C->top()->_idx)->as_JavaObject();
> 59:   // Add ConP(#null) and ConN(#null) nodes.

What does `#NULL` mean?

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

> 2342: }
> 2343: 
> 2344: // Returns unique pointed java object or null.

Use nullptr explicitly here?

src/hotspot/share/opto/gcm.cpp line 287:

> 285:   for (uint k = 0; k < n->len(); k++) { // For all inputs
> 286:     Node* inn = n->in(k);               // Get input
> 287:     if (inn == nullptr)  continue;         // Ignore null, missing inputs

align

src/hotspot/share/opto/gcm.cpp line 1212:

> 1210:       // Bailout without retry
> 1211:       assert(false, "graph should be schedulable");
> 1212:       C->record_method_not_compilable("late schedule failed: LCA == null");

nullptr

src/hotspot/share/opto/generateOptoStub.cpp line 99:

> 97:   // Drop in the last_Java_sp.  last_Java_fp is not touched.
> 98:   // Always do this after the other "last_Java_frame" fields are set since
> 99:   // as soon as last_Java_sp != null the has_last_Java_frame is true and

nullptr

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

> 1608:                                 BasicType bt,
> 1609:                                 DecoratorSet decorators) {
> 1610:   // Transformation of a value which could be null pointer (CastPP #null)

`#NULL` pattern

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

> 2404: // [foo] indicates that 'foo' is a parameter
> 2405: //
> 2406: //              [in]     null

null OK here?

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

> 2742:   // hierarchy and we have to scan the secondary superclass list the hard way.
> 2743:   // Worst-case type is a little odd: null is allowed as a result (usually
> 2744:   // klass loads can never produce a null).

nullptr or null?

src/hotspot/share/opto/ifnode.cpp line 172:

> 170:       //if( vop == Op_Phi ) {     // Phi from another merge point might be OK
> 171:       //  Node *r = v->in(0);     // Get controlling point
> 172:       //  if( !r ) return null;   // Degraded to a copy

nullptr

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

> 2869:   Node* kls = _gvn.transform(LoadKlassNode::make(_gvn, nullptr, immutable_memory(),
> 2870:                                                  basic_plus_adr(cls, java_lang_Class::klass_offset()),
> 2871:                                                  TypeRawPtr::BOTTOM, TypeInstKlassPtr::OBJECT_OR_nullptr));

Faulty macro transform.

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

> 3386:   // different carrier thread, at which point we'll need to use that
> 3387:   // carrier thread's cache.
> 3388:   // return _gvn.transform(LoadNode::make(_gvn, null, immutable_memory(), p, p->bottom_type()->is_ptr(),

nullptr

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 != null (then alloc != null) then we can handle guards and a tightly coupled allocation
> 5142:   // if saved_jvms_before_guards == null and alloc != null, we can't emit any guards

nullptr

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

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

nullptr

src/hotspot/share/opto/loopnode.cpp line 1007:

> 1005:   //
> 1006:   // entry_control: {...}
> 1007:   // long adjusted_limit = limit + stride;  //because phi_incr != null

nullptr

src/hotspot/share/opto/loopnode.cpp line 3409:

> 3407:         *cp = ilt->_next;       // Hang next list at end of child list
> 3408:         *pilt = ilt->_child;    // Move child up to replace ilt
> 3409:         ilt->_head = nullptr;      // Flag as a loop UNIONED into parent

align

src/hotspot/share/opto/loopnode.cpp line 3604:

> 3602:     bool  has_call         = false; // call on dom-path
> 3603:     bool  has_local_ncsfpt = false; // ncsfpt on dom-path at this loop depth
> 3604:     Node* nonlocal_ncsfpt  = nullptr;  // ncsfpt on dom-path at a deeper depth

align

src/hotspot/share/opto/loopnode.cpp line 5301:

> 5299:   } while (l != nullptr);
> 5300:   assert(region->is_in_infinite_subgraph(), "must be in infinite subgraph");
> 5301:   // We have "l->_parent == null", which happens only for infinite loops,

nullptr

src/hotspot/share/opto/machnode.cpp line 303:

> 301:     // Add the offset determined by the "base", or use Type::OffsetBot.
> 302:     if( adr_type == TYPE_PTR_SENTINAL ) {
> 303:       const TypePtr *t_disp = oper->disp_as_type();  // only !null for indOffset32X

not

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

> 956:       return cbuf.oop_recorder()->find_index(_method->constant_encoding());
> 957:     }
> 958:     return 0; // Use symbolic info from bytecode (resolved_method == null).

is

src/hotspot/share/opto/macroArrayCopy.cpp line 1044:

> 1042:   countx = transform_later(new URShiftXNode(countx, intcon(LogBytesPerLong)));
> 1043: 
> 1044:   bool disjoint_bases = true;   // since alloc != null

isn't

src/hotspot/share/opto/matcher.cpp line 328:

> 326:   C->print_method(PHASE_BEFORE_MATCHING, 1);
> 327: 
> 328:   // Create new ideal node ConP #null even if it does exist in old space

`#null` pattern

src/hotspot/share/opto/matcher.cpp line 376:

> 374:       }
> 375: 
> 376:       // Generate new mach node for ConP #null

`#null`

src/hotspot/share/opto/matcher.hpp line 173:

> 171: #endif
> 172: 
> 173:   // Mach node for ConP #null

`#null`

src/hotspot/share/opto/memnode.cpp line 453:

> 451:   // Currently 'sub' is either Allocate, Initialize or Start nodes.
> 452:   // Or Region for the check in LoadNode::Ideal();
> 453:   // 'sub' should have sub->in(0) != null.

nullptr

src/hotspot/share/opto/memnode.cpp line 1120:

> 1118:         Node* st_base = AddPNode::Ideal_base_and_offset(st_adr, phase, st_off);
> 1119:         if (ld_base == nullptr)                                   return nullptr;
> 1120:         if (st_base == nullptr)                                   return nullptr;

align

src/hotspot/share/opto/memnode.cpp line 4632:

> 4630:   init_class_id(Class_MergeMem);
> 4631:   // all inputs are nullified in Node::Node(int)
> 4632:   // set_input(0, null);  // no control input

nullptr

src/hotspot/share/opto/movenode.cpp line 269:

> 267:     case BoolTest::gt: cmp_zero_idx = 2; phi_x_idx = IfTrue;  break;
> 268:     case BoolTest::ge: cmp_zero_idx = 1; phi_x_idx = IfFalse; break;
> 269:     default:           return nullptr;                           break;

align

src/hotspot/share/opto/movenode.cpp line 325:

> 323:     case BoolTest::gt: cmp_zero_idx = 2; phi_x_idx = IfTrue;  break;
> 324:     case BoolTest::ge: cmp_zero_idx = 1; phi_x_idx = IfFalse; break;
> 325:     default:           return nullptr;                           break;

align

src/hotspot/share/opto/movenode.hpp line 42:

> 40:     init_class_id(Class_CMove);
> 41:     // all inputs are nullified in Node::Node(int)
> 42:     // init_req(Control,null);

nullptr

src/hotspot/share/opto/node.cpp line 783:

> 781: 
> 782:   // Find a precedence edge to move
> 783:   if( in(_cnt) != nullptr ) {       // Next precedence edge is busy?

align

src/hotspot/share/opto/node.cpp line 786:

> 784:     uint i;
> 785:     for( i=_cnt; i<_max; i++ )
> 786:       if( in(i) == nullptr )       // Find the null at end of prec edge list

align

src/hotspot/share/opto/node.cpp line 814:

> 812:     uint i;
> 813:     for( i=_cnt; i<_max; i++ )
> 814:       if( _in[i] == nullptr )      // Find the null at end of prec edge list

align

src/hotspot/share/opto/node.cpp line 2076:

> 2074:   tty->print("    else: shortest path or all paths between this/start and target\n");
> 2075:   tty->print("  options:\n");
> 2076:   tty->print("    if null: same as \"cdmox at B\"\n");

null better here imho

src/hotspot/share/opto/node.hpp line 257:

> 255:   // Create a new Node with given input edges.
> 256:   // This version requires use of the "edge-count" new.
> 257:   // E.g.  new (C,3) FooNode( C, null, left, right );

nullptr

src/hotspot/share/opto/node.hpp line 844:

> 842:   }                                                          \
> 843:   type##Node* isa_##type() const {                           \
> 844:     return (is_##type()) ? as_##type() : nullptr;               \

align

src/hotspot/share/opto/node.hpp line 1201:

> 1199:   Node* find_ctrl(int idx); // Search control ancestors for the given idx.
> 1200:   void dump_bfs(const int max_distance, Node* target, const char* options) const; // Print BFS traversal
> 1201:   void dump_bfs(const int max_distance) const; // dump_bfs(max_distance, null, null)

nullptr

src/hotspot/share/opto/node.hpp line 1261:

> 1259: 
> 1260: inline bool not_a_node(const Node* n) {
> 1261:   if (n == nullptr)                   return true;

align

src/hotspot/share/opto/parse1.cpp line 1694:

> 1692:       record_for_igvn(r);
> 1693:       // zap all inputs to null for debugging (done in Node(uint) constructor)
> 1694:       // for (int j = 1; j < edges+1; j++) { r->init_req(j, null); }

nullptr

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

> 108:   if( !k ) {                    // ?Miss?
> 109:     NOT_PRODUCT( _lookup_misses++ );
> 110:     return nullptr;                // Miss!

align

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

> 130:     if( !k ) {                  // ?Miss?
> 131:       NOT_PRODUCT( _lookup_misses++ );
> 132:       return nullptr;              // Miss!

align

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

> 157:     debug_only(n->enter_hash_lock()); // Lock down the node while in the table.
> 158:     check_grow();               // Grow table if insert hit limit
> 159:     return nullptr;                // Miss!

align

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

> 186:       debug_only(n->enter_hash_lock()); // Lock down the node while in the table.
> 187:       check_grow();             // Grow table if insert hit limit
> 188:       return nullptr;              // Miss!

align

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

> 1515:               }
> 1516:             }
> 1517:           } // if (in != null && in != C->top())

nullptr

src/hotspot/share/opto/runtime.cpp line 1550:

> 1548:   SharedRuntime::_rethrow_ctr++;               // count rethrows
> 1549: #endif
> 1550:   assert (exception != nullptr, "should have thrown a nullPointerException");

Weird looking from the start, surely NullPointerException is the correct one?

src/hotspot/share/opto/stringopts.cpp line 130:

> 128:     // Look for a diamond shaped Null check of toString() result
> 129:     // (could be code from String.valueOf()):
> 130:     // (Proj == null) ? "null":"CastPP(Proj)#Notnull

nullptr

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

> 1786:   //    // exception in case X is 0 (because 0-1 turns into 4billion unsigned but
> 1787:   //    // "0 <=u Y" is always true).
> 1788:   //    if( cmp->Opcode() == Op_CmpU ) return null;

nullptr

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

> 75:   _iv(nullptr),                              // induction var
> 76:   _race_possible(false),                  // cases where SDMU is true
> 77:   _early_return(true),                    // analysis evaluations routine

align

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

> 740:         if (best_align_to_mem_ref == nullptr) {
> 741:           if (TraceSuperWord) {
> 742:             tty->print_cr("SuperWord::find_adjacent_refs(): best_align_to_mem_ref == 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 != null: ", _invar->_idx); _invar->dump();

nullptr?

src/hotspot/share/opto/type.hpp line 1695:

> 1693:   // Convenience common pre-built types.
> 1694:   static const TypeInstKlassPtr* OBJECT; // Not-null object klass or below
> 1695:   static const TypeInstKlassPtr* OBJECT_OR_nullptr; // Maybe-null version of same

Macro glitch

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

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