[cleanup] Fix (D)CHECK macros in src/{ast,parsing}

Use the (D)CHECK_{EQ,NE,GT,...} macros instead of (D)CHECK with an
embedded comparison. This gives better error messages and also does the
right comparison for signed/unsigned mismatches.

This will allow us to reenable the readability/check cpplint check.

R=marja@chromium.org

Bug: v8:6837, v8:6921
Change-Id: I17cf5cbbac3d2992c3b3588cc66e8564982453b6
Reviewed-on: https://chromium-review.googlesource.com/681355Reviewed-by: Marja Hölttä <marja@chromium.org>
Commit-Queue: Clemens Hammacher <clemensh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#48596}

src/ast/ast-source-ranges.h

@@ -63,7 +63,7 @@ class ContinuationSourceRanges : public AstNodeSourceRanges {
       : continuation_position_(continuation_position) {}
 
   SourceRange GetRange(SourceRangeKind kind) {
-    DCHECK(kind == SourceRangeKind::kContinuation);
+    DCHECK_EQ(kind, SourceRangeKind::kContinuation);
     return SourceRange::OpenEnded(continuation_position_);
   }
 
@@ -83,7 +83,7 @@ class CaseClauseSourceRanges final : public AstNodeSourceRanges {
       : body_range_(body_range) {}
 
   SourceRange GetRange(SourceRangeKind kind) {
-    DCHECK(kind == SourceRangeKind::kBody);
+    DCHECK_EQ(kind, SourceRangeKind::kBody);
     return body_range_;
   }
 

src/ast/ast-value-factory.cc

@@ -204,7 +204,7 @@ bool AstValue::IsPropertyName() const {
 bool AstValue::BooleanValue() const {
   switch (type_) {
     case STRING:
-      DCHECK(string_ != nullptr);
+      DCHECK_NOT_NULL(string_);
       return !string_->IsEmpty();
     case SYMBOL:
       UNREACHABLE();

src/ast/ast.h

@@ -455,7 +455,7 @@ class FunctionDeclaration final : public Declaration {
 
   FunctionDeclaration(VariableProxy* proxy, FunctionLiteral* fun, int pos)
       : Declaration(proxy, pos, kFunctionDeclaration), fun_(fun) {
-    DCHECK(fun != nullptr);
+    DCHECK_NOT_NULL(fun);
   }
 
   FunctionLiteral* fun_;
@@ -2366,7 +2366,7 @@ class FunctionLiteral final : public Expression {
 
   Handle<String> inferred_name() const {
     if (!inferred_name_.is_null()) {
-      DCHECK(raw_inferred_name_ == nullptr);
+      DCHECK_NULL(raw_inferred_name_);
       return inferred_name_;
     }
     if (raw_inferred_name_ != nullptr) {
@@ -2384,7 +2384,7 @@ class FunctionLiteral final : public Expression {
   }
 
   void set_raw_inferred_name(const AstConsString* raw_inferred_name) {
-    DCHECK(raw_inferred_name != nullptr);
+    DCHECK_NOT_NULL(raw_inferred_name);
     raw_inferred_name_ = raw_inferred_name;
     DCHECK(inferred_name_.is_null());
     inferred_name_ = Handle<String>();

src/ast/prettyprinter.cc

@@ -506,7 +506,7 @@ const char* AstPrinter::Print(AstNode* node) {
 
 void AstPrinter::Init() {
   if (size_ == 0) {
-    DCHECK(output_ == nullptr);
+    DCHECK_NULL(output_);
     const int initial_size = 256;
     output_ = NewArray<char>(initial_size);
     size_ = initial_size;
@@ -644,7 +644,7 @@ AstPrinter::AstPrinter(Isolate* isolate)
 }
 
 AstPrinter::~AstPrinter() {
-  DCHECK(indent_ == 0);
+  DCHECK_EQ(indent_, 0);
   DeleteArray(output_);
 }
 

src/ast/scopes.cc

@@ -100,7 +100,7 @@ Variable* VariableMap::Lookup(const AstRawString* name) {
   Entry* p = ZoneHashMap::Lookup(const_cast<AstRawString*>(name), name->Hash());
   if (p != nullptr) {
     DCHECK(reinterpret_cast<const AstRawString*>(p->key) == name);
-    DCHECK(p->value != nullptr);
+    DCHECK_NOT_NULL(p->value);
     return reinterpret_cast<Variable*>(p->value);
   }
   return nullptr;
@@ -650,7 +650,7 @@ void DeclarationScope::AttachOuterScopeInfo(ParseInfo* info, Isolate* isolate) {
 void DeclarationScope::Analyze(ParseInfo* info) {
   RuntimeCallTimerScope runtimeTimer(info->runtime_call_stats(),
                                      &RuntimeCallStats::CompileScopeAnalysis);
-  DCHECK(info->literal() != nullptr);
+  DCHECK_NOT_NULL(info->literal());
   DeclarationScope* scope = info->literal()->scope();
 
   base::Optional<AllowHandleDereference> allow_deref;
@@ -1096,7 +1096,7 @@ Variable* Scope::DeclareVariable(
          (IsLexicalVariableMode(mode) && is_block_scope()));
 
   VariableProxy* proxy = declaration->proxy();
-  DCHECK(proxy->raw_name() != nullptr);
+  DCHECK_NOT_NULL(proxy->raw_name());
   const AstRawString* name = proxy->raw_name();
   bool is_function_declaration = declaration->IsFunctionDeclaration();
 
@@ -1389,7 +1389,7 @@ bool DeclarationScope::AllowsLazyCompilation() const {
 int Scope::ContextChainLength(Scope* scope) const {
   int n = 0;
   for (const Scope* s = this; s != scope; s = s->outer_scope_) {
-    DCHECK(s != nullptr);  // scope must be in the scope chain
+    DCHECK_NOT_NULL(s);  // scope must be in the scope chain
     if (s->NeedsContext()) n++;
   }
   return n;
@@ -1980,8 +1980,8 @@ void UpdateNeedsHoleCheck(Variable* var, VariableProxy* proxy, Scope* scope) {
   }
 
   // We should always have valid source positions.
-  DCHECK(var->initializer_position() != kNoSourcePosition);
-  DCHECK(proxy->position() != kNoSourcePosition);
+  DCHECK_NE(var->initializer_position(), kNoSourcePosition);
+  DCHECK_NE(proxy->position(), kNoSourcePosition);
 
   if (var->scope()->is_nonlinear() ||
       var->initializer_position() >= proxy->position()) {
@@ -2024,7 +2024,7 @@ void Scope::ResolveVariablesRecursively(ParseInfo* info) {
   // unresolved references remaining, they just need to be resolved in outer
   // scopes.
   if (is_declaration_scope() && AsDeclarationScope()->was_lazily_parsed()) {
-    DCHECK(variables_.occupancy() == 0);
+    DCHECK_EQ(variables_.occupancy(), 0);
     for (VariableProxy* proxy = unresolved_; proxy != nullptr;
          proxy = proxy->next_unresolved()) {
       Variable* var = outer_scope()->LookupRecursive(proxy, nullptr);

src/parsing/func-name-inferrer.cc

@@ -46,7 +46,7 @@ void FuncNameInferrer::PushVariableName(const AstRawString* name) {
 
 void FuncNameInferrer::RemoveAsyncKeywordFromEnd() {
   if (IsOpen()) {
-    CHECK(names_stack_.length() > 0);
+    CHECK_GT(names_stack_.length(), 0);
     CHECK(names_stack_.last().name->IsOneByteEqualTo("async"));
     names_stack_.RemoveLast();
   }

src/parsing/parse-info.cc

@@ -197,7 +197,7 @@ void ParseInfo::ResetCharacterStream() { character_stream_.reset(); }
 
 void ParseInfo::set_character_stream(
     std::unique_ptr<Utf16CharacterStream> character_stream) {
-  DCHECK(character_stream_.get() == nullptr);
+  DCHECK_NULL(character_stream_);
   character_stream_.swap(character_stream);
 }
 

src/parsing/parser-base.h

@@ -1598,7 +1598,7 @@ void ParserBase<Impl>::GetUnexpectedTokenMessage(
       break;
     default:
       const char* name = Token::String(token);
-      DCHECK(name != nullptr);
+      DCHECK_NOT_NULL(name);
       *arg = name;
       break;
   }
@@ -2088,7 +2088,7 @@ template <class Impl>
 typename ParserBase<Impl>::ExpressionT ParserBase<Impl>::ParsePropertyName(
     IdentifierT* name, PropertyKind* kind, bool* is_generator, bool* is_get,
     bool* is_set, bool* is_async, bool* is_computed_name, bool* ok) {
-  DCHECK(*kind == PropertyKind::kNotSet);
+  DCHECK_EQ(*kind, PropertyKind::kNotSet);
   DCHECK(!*is_generator);
   DCHECK(!*is_get);
   DCHECK(!*is_set);
@@ -3051,7 +3051,7 @@ ParserBase<Impl>::ParseConditionalExpression(bool accept_IN,
 template <typename Impl>
 typename ParserBase<Impl>::ExpressionT ParserBase<Impl>::ParseBinaryExpression(
     int prec, bool accept_IN, bool* ok) {
-  DCHECK(prec >= 4);
+  DCHECK_GE(prec, 4);
   ExpressionT x = ParseUnaryExpression(CHECK_OK);
   for (int prec1 = Precedence(peek(), accept_IN); prec1 >= prec; prec1--) {
     // prec1 >= 4
@@ -3784,12 +3784,12 @@ typename ParserBase<Impl>::BlockT ParserBase<Impl>::ParseVariableDeclarations(
       break;
     case Token::CONST:
       Consume(Token::CONST);
-      DCHECK(var_context != kStatement);
+      DCHECK_NE(var_context, kStatement);
       parsing_result->descriptor.mode = CONST;
       break;
     case Token::LET:
       Consume(Token::LET);
-      DCHECK(var_context != kStatement);
+      DCHECK_NE(var_context, kStatement);
       parsing_result->descriptor.mode = LET;
       break;
     default:
@@ -4312,7 +4312,7 @@ ParserBase<Impl>::ParseArrowFunctionLiteral(
         // For arrow functions, we don't need to retrieve data about function
         // parameters.
         int dummy_num_parameters = -1;
-        DCHECK((kind & FunctionKind::kArrowFunction) != 0);
+        DCHECK_NE(kind & FunctionKind::kArrowFunction, 0);
         LazyParsingResult result = impl()->SkipFunction(
             nullptr, kind, FunctionLiteral::kAnonymousExpression,
             formal_parameters.scope, &dummy_num_parameters,

src/parsing/parser.cc

@@ -310,7 +310,7 @@ bool Parser::ShortcutNumericLiteralBinaryExpression(Expression** x,
 
 Expression* Parser::BuildUnaryExpression(Expression* expression,
                                          Token::Value op, int pos) {
-  DCHECK(expression != nullptr);
+  DCHECK_NOT_NULL(expression);
   if (expression->IsLiteral()) {
     const AstValue* literal = expression->AsLiteral()->raw_value();
     if (op == Token::NOT) {
@@ -488,7 +488,7 @@ Parser::Parser(ParseInfo* info)
   // Even though we were passed ParseInfo, we should not store it in
   // Parser - this makes sure that Isolate is not accidentally accessed via
   // ParseInfo during background parsing.
-  DCHECK(info->character_stream() != nullptr);
+  DCHECK_NOT_NULL(info->character_stream());
   // Determine if functions can be lazily compiled. This is necessary to
   // allow some of our builtin JS files to be lazily compiled. These
   // builtins cannot be handled lazily by the parser, since we have to know
@@ -718,7 +718,7 @@ FunctionLiteral* Parser::DoParseProgram(ParseInfo* info) {
   info->set_max_function_literal_id(GetLastFunctionLiteralId());
 
   // Make sure the target stack is empty.
-  DCHECK(target_stack_ == nullptr);
+  DCHECK_NULL(target_stack_);
 
   return result;
 }
@@ -2166,7 +2166,7 @@ Statement* Parser::DesugarLexicalBindingsInForStatement(
   //    }
   //  }
 
-  DCHECK(for_info.bound_names.length() > 0);
+  DCHECK_GT(for_info.bound_names.length(), 0);
   ZoneList<Variable*> temps(for_info.bound_names.length(), zone());
 
   Block* outer_block =
@@ -2240,7 +2240,7 @@ Statement* Parser::DesugarLexicalBindingsInForStatement(
       Statement* assignment_statement =
           factory()->NewExpressionStatement(assignment, kNoSourcePosition);
       int declaration_pos = for_info.parsing_result.descriptor.declaration_pos;
-      DCHECK(declaration_pos != kNoSourcePosition);
+      DCHECK_NE(declaration_pos, kNoSourcePosition);
       decl->proxy()->var()->set_initializer_position(declaration_pos);
       ignore_completion_block->statements()->Add(assignment_statement, zone());
     }
@@ -3349,7 +3349,7 @@ void Parser::UpdateStatistics(Isolate* isolate, Handle<Script> script) {
 void Parser::ParseOnBackground(ParseInfo* info) {
   parsing_on_main_thread_ = false;
 
-  DCHECK(info->literal() == nullptr);
+  DCHECK_NULL(info->literal());
   FunctionLiteral* result = nullptr;
 
   ParserLogger logger;
@@ -4177,7 +4177,7 @@ void Parser::FinalizeIteratorUse(Variable* completion, Expression* condition,
     Block* block = factory()->NewBlock(2, true);
     Expression* proxy = factory()->NewVariableProxy(completion);
     BuildIteratorCloseForCompletion(block->statements(), iter, proxy, type);
-    DCHECK(block->statements()->length() == 2);
+    DCHECK_EQ(block->statements()->length(), 2);
 
     maybe_close = IgnoreCompletion(factory()->NewIfStatement(
         condition, block, factory()->NewEmptyStatement(nopos), nopos));

src/parsing/parser.h

@@ -621,7 +621,7 @@ class V8_EXPORT_PRIVATE Parser : public NON_EXPORTED_BASE(ParserBase<Parser>) {
 
   // Returns true if the expression is of type "this.foo".
   V8_INLINE static bool IsThisProperty(Expression* expression) {
-    DCHECK(expression != nullptr);
+    DCHECK_NOT_NULL(expression);
     Property* property = expression->AsProperty();
     return property != nullptr && property->obj()->IsVariableProxy() &&
            property->obj()->AsVariableProxy()->is_this();
@@ -738,7 +738,7 @@ class V8_EXPORT_PRIVATE Parser : public NON_EXPORTED_BASE(ParserBase<Parser>) {
   // literal so it can be added as a constant function property.
   V8_INLINE static void CheckAssigningFunctionLiteralToProperty(
       Expression* left, Expression* right) {
-    DCHECK(left != nullptr);
+    DCHECK_NOT_NULL(left);
     if (left->IsProperty() && right->IsFunctionLiteral()) {
       right->AsFunctionLiteral()->set_pretenure();
     }
@@ -851,7 +851,7 @@ class V8_EXPORT_PRIVATE Parser : public NON_EXPORTED_BASE(ParserBase<Parser>) {
   // Producing data during the recursive descent.
   V8_INLINE const AstRawString* GetSymbol() const {
     const AstRawString* result = scanner()->CurrentSymbol(ast_value_factory());
-    DCHECK(result != nullptr);
+    DCHECK_NOT_NULL(result);
     return result;
   }
 

src/parsing/pattern-rewriter.cc

@@ -220,7 +220,7 @@ void PatternRewriter::VisitVariableProxy(VariableProxy* pattern) {
   if (!*ok_) return;
   DCHECK_NOT_NULL(var);
   DCHECK(proxy->is_resolved());
-  DCHECK(initializer_position_ != kNoSourcePosition);
+  DCHECK_NE(initializer_position_, kNoSourcePosition);
   var->set_initializer_position(initializer_position_);
 
   Scope* declaration_scope =
@@ -419,7 +419,7 @@ void PatternRewriter::VisitObjectLiteral(ObjectLiteral* pattern,
           DCHECK(key->IsPropertyName() || key->IsNumberLiteral());
         }
 
-        DCHECK(rest_runtime_callargs != nullptr);
+        DCHECK_NOT_NULL(rest_runtime_callargs);
         rest_runtime_callargs->Add(excluded_property, zone());
       }
 

src/parsing/preparsed-scope-data.cc

@@ -397,8 +397,8 @@ void ProducedPreParsedScopeData::SaveDataForInnerScopes(Scope* scope) {
     if (ScopeIsSkippableFunctionScope(inner)) {
       // Don't save data about function scopes, since they'll have their own
       // ProducedPreParsedScopeData where their data is saved.
-      DCHECK(inner->AsDeclarationScope()->produced_preparsed_scope_data() !=
-             nullptr);
+      DCHECK_NOT_NULL(
+          inner->AsDeclarationScope()->produced_preparsed_scope_data());
       continue;
     }
     scopes.push_back(inner);

src/parsing/preparser.h

@@ -400,7 +400,7 @@ inline void PreParserList<PreParserExpression>::Add(
     const PreParserExpression& expression, Zone* zone) {
   if (expression.variables_ != nullptr) {
     DCHECK(FLAG_lazy_inner_functions);
-    DCHECK(zone != nullptr);
+    DCHECK_NOT_NULL(zone);
     if (variables_ == nullptr) {
       variables_ = new (zone) ZoneList<VariableProxy*>(1, zone);
     }

src/parsing/scanner-character-streams.cc

@@ -310,7 +310,7 @@ void Utf8ExternalStreamingStream::FillBufferFromCurrentChunk() {
     unibrow::uchar t =
         unibrow::Utf8::ValueOfIncrementalFinish(&current_.pos.incomplete_char);
     if (t != unibrow::Utf8::kBufferEmpty) {
-      DCHECK(t < unibrow::Utf16::kMaxNonSurrogateCharCode);
+      DCHECK_LT(t, unibrow::Utf16::kMaxNonSurrogateCharCode);
       *cursor = static_cast<uc16>(t);
       buffer_end_++;
       current_.pos.chars++;
@@ -835,9 +835,9 @@ Utf16CharacterStream* ScannerStream::For(Handle<String> data) {
 
 Utf16CharacterStream* ScannerStream::For(Handle<String> data, int start_pos,
                                          int end_pos) {
-  DCHECK(start_pos >= 0);
-  DCHECK(start_pos <= end_pos);
-  DCHECK(end_pos <= data->length());
+  DCHECK_GE(start_pos, 0);
+  DCHECK_LE(start_pos, end_pos);
+  DCHECK_LE(end_pos, data->length());
   if (data->IsExternalOneByteString()) {
     return new ExternalOneByteStringUtf16CharacterStream(
         Handle<ExternalOneByteString>::cast(data),

src/parsing/scanner.cc

@@ -212,7 +212,7 @@ void Scanner::Initialize(Utf16CharacterStream* source, bool is_module) {
 
 template <bool capture_raw, bool unicode>
 uc32 Scanner::ScanHexNumber(int expected_length) {
-  DCHECK(expected_length <= 4);  // prevent overflow
+  DCHECK_LE(expected_length, 4);  // prevent overflow
 
   int begin = source_pos() - 2;
   uc32 x = 0;
@@ -583,7 +583,7 @@ void Scanner::TryToParseSourceURLComment() {
 
 
 Token::Value Scanner::SkipMultiLineComment() {
-  DCHECK(c0_ == '*');
+  DCHECK_EQ(c0_, '*');
   Advance();
 
   while (c0_ != kEndOfInput) {
@@ -609,7 +609,7 @@ Token::Value Scanner::SkipMultiLineComment() {
 
 Token::Value Scanner::ScanHtmlComment() {
   // Check for <!-- comments.
-  DCHECK(c0_ == '!');
+  DCHECK_EQ(c0_, '!');
   Advance();
   if (c0_ != '-') {
     PushBack('!');  // undo Advance()
@@ -1187,8 +1187,8 @@ Token::Value Scanner::ScanTemplateSpan() {
 
 
 Token::Value Scanner::ScanTemplateStart() {
-  DCHECK(next_next_.token == Token::UNINITIALIZED);
-  DCHECK(c0_ == '`');
+  DCHECK_EQ(next_next_.token, Token::UNINITIALIZED);
+  DCHECK_EQ(c0_, '`');
   next_.location.beg_pos = source_pos();
   Advance();  // Consume `
   return ScanTemplateSpan();
@@ -1489,7 +1489,7 @@ uc32 Scanner::ScanUnicodeEscape() {
 
 static Token::Value KeywordOrIdentifierToken(const uint8_t* input,
                                              int input_length) {
-  DCHECK(input_length >= 1);
+  DCHECK_GE(input_length, 1);
   const int kMinLength = 2;
   const int kMaxLength = 11;
   if (input_length < kMinLength || input_length > kMaxLength) {

src/parsing/scanner.h

@@ -414,7 +414,7 @@ class Scanner {
 
     Vector<const uint16_t> two_byte_literal() const {
       DCHECK(!is_one_byte_);
-      DCHECK((position_ & 0x1) == 0);
+      DCHECK_EQ(position_ & 0x1, 0);
       return Vector<const uint16_t>(
           reinterpret_cast<const uint16_t*>(backing_store_.start()),
           position_ >> 1);