RFR: 8300086: Replace NULL with nullptr in share/c1/

Wed May 17 13:41:01 UTC 2023

On Tue, 16 May 2023 12:08:47 GMT, Johan Sjölen <jsjolen at openjdk.org> wrote:

> Hi, this PR changes all occurrences of NULL to nullptr for the subdirectory share/c1. 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:
> 
> 1. No changes but copyright header changed (probably because I reverted some changes but forgot the copyright).
> 2. Macros having their NULL changed to nullptr, these are added to the script when I find them. They should be NULL.
> 3. 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:
> 
> ```c++
> // 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!

I only have some minor code style things. Otherwise, looks good to me, too!

src/hotspot/share/c1/c1_GraphBuilder.cpp line 3421:

> 3419: 
> 3420: # ifdef ASSERT
> 3421:   //All blocks reachable from start_block have _end isn't null

Suggestion:

  // For all blocks reachable from start_block: _end must be non-null

src/hotspot/share/c1/c1_Instruction.cpp line 103:

> 101:     state_before()->values_do(f);
> 102:   }
> 103:   if (exception_state() != nullptr){

Suggestion:

  if (exception_state() != nullptr) {

src/hotspot/share/c1/c1_LIR.cpp line 513:

> 511:       if (opBranch->_opr2->is_valid()) do_input(opBranch->_opr2);
> 512: 
> 513:       if (opBranch->_info != nullptr)     do_info(opBranch->_info);

Maybe keep previous alignment:
Suggestion:

      if (opBranch->_info != nullptr)  do_info(opBranch->_info);

src/hotspot/share/c1/c1_LIR.cpp line 515:

> 513:       if (opBranch->_info != nullptr)     do_info(opBranch->_info);
> 514:       assert(opBranch->_result->is_illegal(), "not used");
> 515:       if (opBranch->_stub != nullptr)     opBranch->stub()->visit(this);

Maybe keep previous alignment:
Suggestion:

      if (opBranch->_stub != nullptr)  opBranch->stub()->visit(this);

src/hotspot/share/c1/c1_LIR.cpp line 2074:

> 2072: void LIR_OpProfileType::print_instr(outputStream* out) const {
> 2073:   out->print("exact = ");
> 2074:   if  (exact_klass() == nullptr) {

Suggestion:

  if (exact_klass() == nullptr) {

src/hotspot/share/c1/c1_LinearScan.cpp line 2983:

> 2981:   for (int j = 0; j < num_inst; j++) {
> 2982:     LIR_Op* op = instructions->at(j);
> 2983:     if (op == nullptr) {  // this can happen when spill-moves are removed in eliminate_spill_moves

Suggestion:

    if (op == nullptr) { // this can happen when spill-moves are removed in eliminate_spill_moves

src/hotspot/share/c1/c1_Optimizer.cpp line 319:

> 317:     } else {
> 318:       Constant* x_const = x->as_Constant();
> 319:       if (x_const != nullptr) {         // x and y are constants

Suggestion:

      if (x_const != nullptr) { // x and y are constants

src/hotspot/share/c1/c1_RangeCheckElimination.cpp line 137:

> 135:   bool has_lower = true;
> 136:   assert(phi, "Phi must not be null");
> 137:   Bound *bound = nullptr;

Suggestion:

  Bound* bound = nullptr;

src/hotspot/share/c1/c1_RangeCheckElimination.cpp line 368:

> 366:   assert(loop_header, "Loop header must not be null!");
> 367:   if (!instruction) return true;
> 368:   for (BlockBegin *d = loop_header->dominator(); d != nullptr; d = d->dominator()) {

Suggestion:

  for (BlockBegin* d = loop_header->dominator(); d != nullptr; d = d->dominator()) {

src/hotspot/share/c1/c1_RangeCheckElimination.cpp line 386:

> 384:     assert(_bounds.at(v->id()), "Now Stack must exist");
> 385:   }
> 386:   Bound *top = nullptr;

Suggestion:

  Bound* top = nullptr;

src/hotspot/share/c1/c1_RangeCheckElimination.cpp line 954:

> 952:       cur_value = nullptr;
> 953:     }
> 954:     Bound *new_index_bound = new Bound(0, nullptr, cur_constant, cur_value);

Suggestion:

    Bound* new_index_bound = new Bound(0, nullptr, cur_constant, cur_value);

src/hotspot/share/c1/c1_RangeCheckElimination.cpp line 1102:

> 1100:     for (int i=0; i<block->number_of_sux(); i++) {
> 1101:       BlockBegin *sux = block->sux_at(i);
> 1102:       BlockBegin *pred = nullptr;

Suggestion:

      BlockBegin* pred = nullptr;

src/hotspot/share/c1/c1_RangeCheckElimination.cpp line 1224:

> 1222: 
> 1223: // Try to reach Block end beginning in Block start and not using Block dont_use
> 1224: bool RangeCheckEliminator::Verification::can_reach(BlockBegin *start, BlockBegin *end, BlockBegin *dont_use /* = nullptr */) {

Suggestion:

bool RangeCheckEliminator::Verification::can_reach(BlockBegin* start, BlockBegin* end, BlockBegin* dont_use /* = nullptr */) {

src/hotspot/share/c1/c1_RangeCheckElimination.cpp line 1522:

> 1520: void RangeCheckEliminator::Bound::add_assertion(Instruction *instruction, Instruction *position, int i, Value instr, Instruction::Condition cond) {
> 1521:   Instruction *result = position;
> 1522:   Instruction *compare_with = nullptr;

Suggestion:

  Instruction* compare_with = nullptr;

src/hotspot/share/c1/c1_RangeCheckElimination.cpp line 1533:

> 1531:   result = instruction_before;
> 1532:   // Load constant only if needed
> 1533:   Constant *constant = nullptr;

Suggestion:

  Constant* constant = nullptr;

src/hotspot/share/c1/c1_RangeCheckElimination.cpp line 1557:

> 1555:     // Add operation only if necessary
> 1556:     if (constant) {
> 1557:       ArithmeticOp *ao = new ArithmeticOp(Bytecodes::_iadd, constant, op, nullptr);

Suggestion:

      ArithmeticOp* ao = new ArithmeticOp(Bytecodes::_iadd, constant, op, nullptr);

src/hotspot/share/c1/c1_Runtime1.cpp line 960:

> 958:   Klass* load_klass = nullptr; // klass needed by load_klass_patching code
> 959:   Handle mirror(current, nullptr);                    // oop needed by load_mirror_patching code
> 960:   Handle appendix(current, nullptr);                  // oop needed by appendix_patching code

Strange alignment
Suggestion:

  Handle mirror(current, nullptr); // oop needed by load_mirror_patching code
  Handle appendix(current, nullptr); // oop needed by appendix_patching code

src/hotspot/share/c1/c1_ValueMap.hpp line 246:

> 244:   ValueMap*     current_map()                    { return _current_map; }
> 245:   ValueMap*     value_map_of(BlockBegin* block)  { return _value_maps.at(block->linear_scan_number()); }
> 246:   void          set_value_map_of(BlockBegin* block, ValueMap* map)   { assert(value_map_of(block) == nullptr, ""); _value_maps.at_put(block->linear_scan_number(), map); }

Suggestion:

  void          set_value_map_of(BlockBegin* block, ValueMap* map) { assert(value_map_of(block) == nullptr, ""); _value_maps.at_put(block->linear_scan_number(), map); }

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

Marked as reviewed by chagedorn (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/14009#pullrequestreview-1430621110
PR Review Comment: https://git.openjdk.org/jdk/pull/14009#discussion_r1196473301
PR Review Comment: https://git.openjdk.org/jdk/pull/14009#discussion_r1196476625
PR Review Comment: https://git.openjdk.org/jdk/pull/14009#discussion_r1196529402
PR Review Comment: https://git.openjdk.org/jdk/pull/14009#discussion_r1196529703
PR Review Comment: https://git.openjdk.org/jdk/pull/14009#discussion_r1196492675
PR Review Comment: https://git.openjdk.org/jdk/pull/14009#discussion_r1196507880
PR Review Comment: https://git.openjdk.org/jdk/pull/14009#discussion_r1196515212
PR Review Comment: https://git.openjdk.org/jdk/pull/14009#discussion_r1196516467
PR Review Comment: https://git.openjdk.org/jdk/pull/14009#discussion_r1196516995
PR Review Comment: https://git.openjdk.org/jdk/pull/14009#discussion_r1196517209
PR Review Comment: https://git.openjdk.org/jdk/pull/14009#discussion_r1196517785
PR Review Comment: https://git.openjdk.org/jdk/pull/14009#discussion_r1196518124
PR Review Comment: https://git.openjdk.org/jdk/pull/14009#discussion_r1196518753
PR Review Comment: https://git.openjdk.org/jdk/pull/14009#discussion_r1196519435
PR Review Comment: https://git.openjdk.org/jdk/pull/14009#discussion_r1196519874
PR Review Comment: https://git.openjdk.org/jdk/pull/14009#discussion_r1196520284
PR Review Comment: https://git.openjdk.org/jdk/pull/14009#discussion_r1196522421
PR Review Comment: https://git.openjdk.org/jdk/pull/14009#discussion_r1196524998