Avoid comparison against nullptr, instead use it as boolean identity.

.github/bin/check-potential-problems.sh

@@ -71,10 +71,22 @@ if [ $? -eq 0 ]; then
   EXIT_CODE=1
 fi
 
+# Improve readability of pointer comparisons. We always use pointers in a
+# 'does this thing (not) exist' context, and comparing it as boolean makes
+# such expressions read more naturally.
+find common verilog -name "*.h" -o -name "*.cc" | \
+  xargs egrep -n ' [=!]= nullptr' | grep -v "// NOLINT"
+if [ $? -eq 0 ]; then
+  echo "::error:: Avoid comparing against nullptr, use boolean identiy instead."
+  echo
+  EXIT_CODE=1
+fi
+
 # bazelbuild/rules_python is broken as it downloads a dynamically
 # linked pre-built binary - This makes it _very_ platform specific.
 # This should either compile Python from scratch or use the local system Python.
-# So before rules_python() is added here, this needs to be fixed first upstream.
+# So if Python is ever needed and before rules_python() is added here, this
+# needs to be fixed first upstream.
 # https://github.com/bazelbuild/rules_python/issues/1211
 grep rules_python WORKSPACE* MODULE.bazel
 if [ $? -eq 0 ]; then

common/analysis/lint_waiver.cc

@@ -84,7 +84,7 @@ void LintWaiver::RegexToLines(absl::string_view contents,
 bool LintWaiver::RuleIsWaivedOnLine(absl::string_view rule_name,
                                     int line_number) const {
   const auto *line_set = verible::container::FindOrNull(waiver_map_, rule_name);
-  return line_set != nullptr && LineNumberSetContains(*line_set, line_number);
+  return line_set && LineNumberSetContains(*line_set, line_number);
 }
 
 bool LintWaiver::Empty() const {

common/analysis/matcher/matcher_test_utils.h

@@ -53,7 +53,7 @@ void RunRawMatcherTestCase(const RawMatcherTestCase &test) {
   EXPECT_TRUE(status.ok()) << "code with error:\n" << test.code;
 
   auto *tree = analyzer.SyntaxTree().get();
-  EXPECT_TRUE(tree != nullptr);
+  EXPECT_TRUE(tree);
 
   ExpectMatchesInAST(*tree, test.matcher, test.num_matches, test.code);
 }

common/analysis/syntax_tree_search_test_utils.cc

@@ -75,7 +75,7 @@ bool SyntaxTreeSearchTestCase::ExactMatchFindings(
   // Convert actual_findings into string ranges.  Ignore matches' context.
   StringRangeSet actual_findings_ranges;
   for (const auto &finding : actual_findings) {
-    if (finding.match == nullptr) continue;
+    if (!finding.match) continue;
     const auto &match_symbol(*finding.match);
     const absl::string_view spanned_text = StringSpanOfSymbol(match_symbol);
     // Spanned text can be empty when a subtree is devoid of leaves.

common/formatting/align.cc

@@ -325,8 +325,8 @@ struct AlignedColumnConfiguration {
   // constructs.
   const SyntaxTreeLeaf* leaf = GetLeftmostLeaf(symbol);
   // It is possible for a node to be empty, in which case, ignore.
-  if (leaf == nullptr) return nullptr;
-  if (parent_column->Parent() != nullptr && parent_column->Children().empty()) {
+  if (!leaf) return nullptr;
+  if (parent_column->Parent() && parent_column->Children().empty()) {
     // Starting token of a column and its first subcolumn must be the same.
     // (subcolumns overlap their parent column).
     CHECK_EQ(parent_column->Value().starting_token, leaf->get());
@@ -559,7 +559,7 @@ static void FillAlignmentRow(
       CHECK(token_iter != remaining_tokens_range.end());
       remaining_tokens_range.set_begin(token_iter);
 
-      if (prev_cell_tokens != nullptr) prev_cell_tokens->set_end(token_iter);
+      if (prev_cell_tokens) prev_cell_tokens->set_end(token_iter);
 
       AlignmentRow& row_cell = verible::DescendPath(
           *row, column_loc_iter->second.begin(), column_loc_iter->second.end());

common/formatting/align.h

@@ -343,7 +343,7 @@ ColumnPositionTree ScanPartitionForAlignmentCells(
   // this 'row', and detect the sparse set of columns found by the scanner.
   ScannerType scanner = scanner_factory();
   const Symbol *origin = unwrapped_line.Origin();
-  if (origin != nullptr) origin->Accept(&scanner);
+  if (origin) origin->Accept(&scanner);
   return scanner.SparseColumns();
 }
 
@@ -385,7 +385,7 @@ ColumnPositionTree ScanPartitionForAlignmentCells_WithNonTreeTokens(
 
   FormatTokenRange leading_tokens(ftokens.begin(), ftokens.begin());
   FormatTokenRange trailing_tokens(ftokens.end(), ftokens.end());
-  if (origin != nullptr) {
+  if (origin) {
     // Identify the last token covered by the origin tree.
     const SyntaxTreeLeaf *first_leaf = GetLeftmostLeaf(*origin);
     const SyntaxTreeLeaf *last_leaf = GetRightmostLeaf(*origin);

common/formatting/align_test.cc

@@ -1089,7 +1089,7 @@ class SubcolumnsTreeAlignmentTest : public MatrixTreeAlignmentTestFixture {
       const std::vector<verible::PreFormatToken>::iterator& end) {
     SymbolPtr list = TNode(0);
     SymbolPtr item;
-    while ((item = ParseItem(it, end)) != nullptr) {
+    while ((item = ParseItem(it, end))) {
       SymbolCastToNode(*list).AppendChild(std::move(item));
     }
     return list;

common/formatting/format_token.cc

@@ -74,11 +74,11 @@ std::ostream &operator<<(std::ostream &stream, GroupBalancing b) {
 }
 
 std::ostream &operator<<(std::ostream &stream, const InterTokenInfo &t) {
+  const int print_preserve_space = t.preserved_space_start ? 1 : 0;
   stream << "{\n  spaces_required: " << t.spaces_required
          << "\n  break_penalty: " << t.break_penalty
          << "\n  break_decision: " << t.break_decision
-         << "\n  preserve_space?: " << (t.preserved_space_start != nullptr)
-         << "\n}";
+         << "\n  preserve_space?: " << print_preserve_space << "\n}";
   return stream;
 }
 
@@ -145,7 +145,7 @@ InterTokenDecision::InterTokenDecision(const InterTokenInfo &info)
 
 static absl::string_view OriginalLeadingSpacesRange(const char *begin,
                                                     const char *end) {
-  if (begin == nullptr) {
+  if (!begin) {
     VLOG(4) << "no original space range";
     return make_string_view_range(end, end);  // empty range
   }
@@ -163,7 +163,7 @@ absl::string_view FormattedToken::OriginalLeadingSpaces() const {
 std::ostream &FormattedToken::FormattedText(std::ostream &stream) const {
   switch (before.action) {
     case SpacingDecision::kPreserve: {
-      if (before.preserved_space_start != nullptr) {
+      if (before.preserved_space_start) {
         // Calculate string_view range of pre-existing spaces, and print that.
         stream << OriginalLeadingSpaces();
       } else {
@@ -196,15 +196,15 @@ absl::string_view PreFormatToken::OriginalLeadingSpaces() const {
 
 size_t PreFormatToken::LeadingSpacesLength() const {
   if (before.break_decision == SpacingOptions::kPreserve &&
-      before.preserved_space_start != nullptr) {
+      before.preserved_space_start) {
     return OriginalLeadingSpaces().length();
   }
   // in other cases (append, wrap), take the spaces_required value.
   return before.spaces_required;
 }
 
 int PreFormatToken::ExcessSpaces() const {
-  if (before.preserved_space_start == nullptr) return 0;
+  if (!before.preserved_space_start) return 0;
   const absl::string_view leading_spaces = OriginalLeadingSpaces();
   int delta = 0;
   if (!absl::StrContains(leading_spaces, "\n")) {
@@ -230,7 +230,7 @@ std::ostream &operator<<(std::ostream &stream, const PreFormatToken &t) {
 void ConnectPreFormatTokensPreservedSpaceStarts(
     const char *buffer_start, std::vector<PreFormatToken> *format_tokens) {
   VLOG(4) << __FUNCTION__;
-  CHECK(buffer_start != nullptr);
+  CHECK(buffer_start);
   for (auto &ftoken : *format_tokens) {
     ftoken.before.preserved_space_start = buffer_start;
     VLOG(4) << "space: " << VisualizeWhitespace(ftoken.OriginalLeadingSpaces());

common/formatting/format_token_test.cc

@@ -151,7 +151,7 @@ TEST(PreFormatTokenTest, OriginalLeadingSpaces) {
 TEST(PreFormatTokenTest, ExcessSpacesNoNewline) {
   const absl::string_view text("abcdefgh");
   const TokenInfo tok(1, text.substr(1, 3));
-  PreFormatToken p(&tok);  // before.preserved_space_start == nullptr
+  PreFormatToken p(&tok);  //! before.preserved_space_start
   EXPECT_EQ(p.ExcessSpaces(), 0);
 
   p.before.preserved_space_start = text.begin();
@@ -165,7 +165,7 @@ TEST(PreFormatTokenTest, ExcessSpacesNoNewline) {
 TEST(PreFormatTokenTest, ExcessSpacesNewline) {
   const absl::string_view text("\nbcdefgh");
   const TokenInfo tok(1, text.substr(1, 3));
-  PreFormatToken p(&tok);  // before.preserved_space_start == nullptr
+  PreFormatToken p(&tok);  //! before.preserved_space_start
   EXPECT_EQ(p.ExcessSpaces(), 0);
 
   p.before.preserved_space_start = text.begin();

common/formatting/layout_optimizer.cc

@@ -678,14 +678,14 @@ void TreeReconstructor::TraverseTree(const LayoutTree& layout_tree) {
   // Setting indentation for a line that is going to be appended is invalid and
   // probably has been done for some reason that is not going to work as
   // intended.
-  LOG_IF(WARNING, ((relative_indentation > 0) && (current_node_ != nullptr)))
+  LOG_IF(WARNING, ((relative_indentation > 0) && current_node_))
       << "Discarding indentation of a line that's going to be appended.";
 
   switch (layout.Type()) {
     case LayoutType::kLine: {
       CHECK(layout_tree.Children().empty());
 
-      if (current_node_ == nullptr) {
+      if (!current_node_) {
         auto uwline = UnwrappedLine(current_indentation_spaces_,
                                     layout.TokensRange().begin(),
                                     PartitionPolicyEnum::kAlreadyFormatted);
@@ -744,7 +744,7 @@ void TreeReconstructor::TraverseTree(const LayoutTree& layout_tree) {
 
       // Calculate indent for 2nd and further lines.
       int indentation = current_indentation_spaces_;
-      if (current_node_ != nullptr) {
+      if (current_node_) {
         indentation = AlreadyFormattedPartitionLength(*current_node_) +
                       layout.SpacesBefore();
       }

common/formatting/layout_optimizer_test.cc

@@ -108,7 +108,7 @@ void ExpectLayoutFunctionsEqual(const LayoutFunction& actual,
                                expected[i].span, 2);
     }
     auto layout_diff = DeepEqual(actual[i].layout, expected[i].layout);
-    if (layout_diff.left != nullptr) {
+    if (layout_diff.left) {
       PrintInvalidValueMessage(segment_msg, "layout (fragment)",
                                *layout_diff.left, *layout_diff.right, 2, true);
     }

common/formatting/state_node.h

@@ -113,7 +113,7 @@ struct StateNode {
 
   // Returns true if this state was initialized with an unwrapped line and
   // has no parent state.
-  bool IsRootState() const { return prev_state == nullptr; }
+  bool IsRootState() const { return !prev_state; }
 
   // Returns the total number of nodes in state ancestry, including itself.
   // This occurs in O(N) time, and is only suitable for testing/debug.

common/formatting/token_partition_tree.cc

@@ -153,7 +153,7 @@ std::ostream& TokenPartitionTreePrinter::PrintTree(std::ostream& stream,
            // <auto> just means the concatenation of all subpartitions
            << "[<auto>], policy: " << value.PartitionPolicy() << ") @"
            << NodePath(node);
-    if (value.Origin() != nullptr) {
+    if (value.Origin()) {
       stream << ", (origin: ";
       origin_printer(stream, value.Origin());
       stream << ")";
@@ -384,8 +384,7 @@ void MergeConsecutiveSiblings(TokenPartitionTree* tree, size_t pos) {
 static void UpdateTokenRangeLowerBound(TokenPartitionTree* leaf,
                                        TokenPartitionTree* last,
                                        format_token_iterator token_iter) {
-  for (auto* node = leaf; node != nullptr && node != last;
-       node = node->Parent()) {
+  for (auto* node = leaf; node && node != last; node = node->Parent()) {
     node->Value().SpanBackToToken(token_iter);
   }
 }
@@ -395,8 +394,7 @@ static void UpdateTokenRangeLowerBound(TokenPartitionTree* leaf,
 static void UpdateTokenRangeUpperBound(TokenPartitionTree* leaf,
                                        TokenPartitionTree* last,
                                        format_token_iterator token_iter) {
-  for (auto* node = leaf; node != nullptr && node != last;
-       node = node->Parent()) {
+  for (auto* node = leaf; node && node != last; node = node->Parent()) {
     node->Value().SpanUpToToken(token_iter);
   }
 }
@@ -405,7 +403,7 @@ TokenPartitionTree* GroupLeafWithPreviousLeaf(TokenPartitionTree* leaf) {
   CHECK_NOTNULL(leaf);
   VLOG(4) << "origin leaf:\n" << *leaf;
   auto* previous_leaf = PreviousLeaf(*leaf);
-  if (previous_leaf == nullptr) return nullptr;
+  if (!previous_leaf) return nullptr;
   VLOG(4) << "previous leaf:\n" << *previous_leaf;
 
   // If there is no common ancestor, do nothing and return.
@@ -455,7 +453,7 @@ TokenPartitionTree* MergeLeafIntoPreviousLeaf(TokenPartitionTree* leaf) {
   CHECK_NOTNULL(leaf);
   VLOG(4) << "origin leaf:\n" << *leaf;
   auto* target_leaf = PreviousLeaf(*leaf);
-  if (target_leaf == nullptr) return nullptr;
+  if (!target_leaf) return nullptr;
   VLOG(4) << "target leaf:\n" << *target_leaf;
 
   // If there is no common ancestor, do nothing and return.
@@ -501,7 +499,7 @@ TokenPartitionTree* MergeLeafIntoNextLeaf(TokenPartitionTree* leaf) {
   CHECK_NOTNULL(leaf);
   VLOG(4) << "origin leaf:\n" << *leaf;
   auto* target_leaf = NextLeaf(*leaf);
-  if (target_leaf == nullptr) return nullptr;
+  if (!target_leaf) return nullptr;
   VLOG(4) << "target leaf:\n" << *target_leaf;
 
   // If there is no common ancestor, do nothing and return.
@@ -565,7 +563,7 @@ class TokenPartitionTreeWrapper {
   TokenPartitionTreeWrapper() = delete;
 
   TokenPartitionTreeWrapper(const TokenPartitionTreeWrapper& other) {
-    CHECK((other.node_ != nullptr) || (other.unwrapped_line_ != nullptr));
+    CHECK(other.node_ || other.unwrapped_line_);
 
     if (other.node_) {
       node_ = other.node_;
@@ -582,7 +580,7 @@ class TokenPartitionTreeWrapper {
 
   // Concatenate subnodes value with other node value
   UnwrappedLine Value(const TokenPartitionTree& other) const {
-    CHECK((node_ == nullptr) && (unwrapped_line_ != nullptr));
+    CHECK(!node_ && unwrapped_line_);
     UnwrappedLine uw = *unwrapped_line_;
     uw.SpanUpToToken(other.Value().TokensRange().end());
     return uw;
@@ -598,7 +596,7 @@ class TokenPartitionTreeWrapper {
 
   // Fix concatenated value indentation
   void SetIndentationSpaces(int indent) {
-    CHECK((node_ == nullptr) && (unwrapped_line_ != nullptr));
+    CHECK(!node_ && unwrapped_line_);
     unwrapped_line_->SetIndentationSpaces(indent);
   }