[parser] More CHECK_OK cleanup.

src/parsing/parser.cc

 // Copyright 2012 the V8 project authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "src/parsing/parser.h"
 
 #include "src/api.h"
 #include "src/ast/ast.h"
 #include "src/ast/ast-expression-rewriter.h"
 #include "src/ast/ast-expression-visitor.h"
@@ skipping 290 matching lines @@
 
 
 // ----------------------------------------------------------------------------
 // The CHECK_OK macro is a convenient macro to enforce error
 // handling for functions that may fail (by returning !*ok).
 //
 // CAUTION: This macro appends extra statements after a call,
 // thus it must never be used where only a single statement
 // is correct (e.g. an if statement branch w/o braces)!
 
-#define CHECK_OK  ok);   \
-  if (!*ok) return NULL; \
+#define CHECK_OK  ok);      \
+  if (!*ok) return nullptr; \
   ((void)0
 #define DUMMY )  // to make indentation work
 #undef DUMMY
 
-// Used in functions where the return type is not ExpressionT.
-#define CHECK_OK_CUSTOM(x) ok); \
-  if (!*ok) return this->x();   \
+#define CHECK_OK_VOID  ok); \
+  if (!*ok) return;         \
   ((void)0
 #define DUMMY )  // to make indentation work
 #undef DUMMY
 
 #define CHECK_FAILED /**/); \
   if (failed_) return NULL;  \

[nickie, 2016/07/19 10:56:07]

Well, since you changed NULL to nullptr in line 312...  :-)

[neis, 2016/07/19 15:21:21]

Done.

   ((void)0
 #define DUMMY )  // to make indentation work
 #undef DUMMY
 
 // ----------------------------------------------------------------------------
 // Implementation of Parser
 
 bool ParserTraits::IsEval(const AstRawString* identifier) const {
   return identifier == parser_->ast_value_factory()->eval_string();
 }
@@ skipping 858 matching lines @@
   DCHECK(target_stack_ == NULL);
 
   if (result != NULL) {
     Handle<String> inferred_name(shared_info->inferred_name());
     result->set_inferred_name(inferred_name);
   }
   return result;
 }
 
 
-void* Parser::ParseStatementList(ZoneList<Statement*>* body, int end_token,
-                                 bool* ok) {
+void Parser::ParseStatementList(ZoneList<Statement*>* body, int end_token,
+                                bool* ok) {
   // StatementList ::
   //   (StatementListItem)* <end_token>
 
   // Allocate a target stack to use for this set of source
   // elements. This way, all scripts and functions get their own
   // target stack thus avoiding illegal breaks and continues across
   // functions.
   TargetScope scope(&this->target_stack_);
 
   DCHECK(body != NULL);
   bool directive_prologue = true;     // Parsing directive prologue.
 
   while (peek() != end_token) {
     if (directive_prologue && peek() != Token::STRING) {
       directive_prologue = false;
     }
 
     Scanner::Location token_loc = scanner()->peek_location();
-    Statement* stat = ParseStatementListItem(CHECK_OK);
+    Statement* stat = ParseStatementListItem(CHECK_OK_VOID);
     if (stat == NULL || stat->IsEmpty()) {
       directive_prologue = false;   // End of directive prologue.
       continue;
     }
 
     if (directive_prologue) {
       // A shot at a directive.
       ExpressionStatement* e_stat;
       Literal* literal;
       // Still processing directive prologue?
@@ skipping 13 matching lines @@
 
           if (!scope_->HasSimpleParameters()) {
             // TC39 deemed "use strict" directives to be an error when occurring
             // in the body of a function with non-simple parameter list, on
             // 29/7/2015. https://goo.gl/ueA7Ln
             const AstRawString* string = literal->raw_value()->AsString();
             ParserTraits::ReportMessageAt(
                 token_loc, MessageTemplate::kIllegalLanguageModeDirective,
                 string);
             *ok = false;
-            return nullptr;
+            return;
           }
           // Because declarations in strict eval code don't leak into the scope
           // of the eval call, it is likely that functions declared in strict
           // eval code will be used within the eval code, so lazy parsing is
           // probably not a win.
           if (scope_->is_eval_scope()) mode_ = PARSE_EAGERLY;
         } else if (literal->raw_value()->AsString() ==
                        ast_value_factory()->use_asm_string() &&
                    token_loc.end_pos - token_loc.beg_pos ==
                        ast_value_factory()->use_asm_string()->length() + 2) {
@@ skipping 10 matching lines @@
         // End of the directive prologue.
         directive_prologue = false;
         RaiseLanguageMode(SLOPPY);
       }
     } else {
       RaiseLanguageMode(SLOPPY);
     }
 
     body->Add(stat, zone());
   }
-
-  return 0;
 }
 
 
 Statement* Parser::ParseStatementListItem(bool* ok) {
   // (Ecma 262 6th Edition, 13.1):
   // StatementListItem:
   //    Statement
   //    Declaration
   const Token::Value peeked = peek();
   switch (peeked) {
@@ skipping 37 matching lines @@
       ParseImportDeclaration(CHECK_OK);
       return factory()->NewEmptyStatement(kNoSourcePosition);
     case Token::EXPORT:
       return ParseExportDeclaration(ok);
     default:
       return ParseStatementListItem(ok);
   }
 }
 
 
-void* Parser::ParseModuleItemList(ZoneList<Statement*>* body, bool* ok) {
+void Parser::ParseModuleItemList(ZoneList<Statement*>* body, bool* ok) {
   // ecma262/#prod-Module
   // Module :
   //    ModuleBody?
   //
   // ecma262/#prod-ModuleItemList
   // ModuleBody :
   //    ModuleItem*
 
   DCHECK(scope_->is_module_scope());
   while (peek() != Token::EOS) {
-    Statement* stat = ParseModuleItem(CHECK_OK);
+    Statement* stat = ParseModuleItem(CHECK_OK_VOID);
     if (stat && !stat->IsEmpty()) {
       body->Add(stat, zone());
     }
   }
-  return nullptr;
 }
 
 
 const AstRawString* Parser::ParseModuleSpecifier(bool* ok) {
   // ModuleSpecifier :
   //    StringLiteral
 
   Expect(Token::STRING, CHECK_OK);
   return GetSymbol(scanner());
 }
 
 
-void* Parser::ParseExportClause(ZoneList<const AstRawString*>* export_names,
-                                ZoneList<Scanner::Location>* export_locations,
-                                ZoneList<const AstRawString*>* local_names,
-                                Scanner::Location* reserved_loc, bool* ok) {
+void Parser::ParseExportClause(ZoneList<const AstRawString*>* export_names,
+                               ZoneList<Scanner::Location>* export_locations,
+                               ZoneList<const AstRawString*>* local_names,
+                               Scanner::Location* reserved_loc, bool* ok) {
   // ExportClause :
   //   '{' '}'
   //   '{' ExportsList '}'
   //   '{' ExportsList ',' '}'
   //
   // ExportsList :
   //   ExportSpecifier
   //   ExportsList ',' ExportSpecifier
   //
   // ExportSpecifier :
   //   IdentifierName
   //   IdentifierName 'as' IdentifierName
 
-  Expect(Token::LBRACE, CHECK_OK);
+  Expect(Token::LBRACE, CHECK_OK_VOID);
 
   Token::Value name_tok;
   while ((name_tok = peek()) != Token::RBRACE) {
     // Keep track of the first reserved word encountered in case our
     // caller needs to report an error.
     if (!reserved_loc->IsValid() &&
         !Token::IsIdentifier(name_tok, STRICT, false, parsing_module_)) {
       *reserved_loc = scanner()->location();
     }
-    const AstRawString* local_name = ParseIdentifierName(CHECK_OK);
+    const AstRawString* local_name = ParseIdentifierName(CHECK_OK_VOID);
     const AstRawString* export_name = NULL;
     if (CheckContextualKeyword(CStrVector("as"))) {
-      export_name = ParseIdentifierName(CHECK_OK);
+      export_name = ParseIdentifierName(CHECK_OK_VOID);
     }
     if (export_name == NULL) {
       export_name = local_name;
     }
     export_names->Add(export_name, zone());
     local_names->Add(local_name, zone());
     export_locations->Add(scanner()->location(), zone());
     if (peek() == Token::RBRACE) break;
-    Expect(Token::COMMA, CHECK_OK);
+    Expect(Token::COMMA, CHECK_OK_VOID);
   }
 
-  Expect(Token::RBRACE, CHECK_OK);
-
-  return nullptr;
+  Expect(Token::RBRACE, CHECK_OK_VOID);
 }
 
 
 ZoneList<const Parser::NamedImport*>* Parser::ParseNamedImports(
     int pos, bool* ok) {
   // NamedImports :
   //   '{' '}'
   //   '{' ImportsList '}'
   //   '{' ImportsList ',' '}'
   //
@@ skipping 36 matching lines @@
 
     if (peek() == Token::RBRACE) break;
     Expect(Token::COMMA, CHECK_OK);
   }
 
   Expect(Token::RBRACE, CHECK_OK);
   return result;
 }
 
 
-void* Parser::ParseImportDeclaration(bool* ok) {
+void Parser::ParseImportDeclaration(bool* ok) {
   // ImportDeclaration :
   //   'import' ImportClause 'from' ModuleSpecifier ';'
   //   'import' ModuleSpecifier ';'
   //
   // ImportClause :
   //   ImportedDefaultBinding
   //   NameSpaceImport
   //   NamedImports
   //   ImportedDefaultBinding ',' NameSpaceImport
   //   ImportedDefaultBinding ',' NamedImports
   //
   // NameSpaceImport :
   //   '*' 'as' ImportedBinding
 
   int pos = peek_position();
-  Expect(Token::IMPORT, CHECK_OK);
+  Expect(Token::IMPORT, CHECK_OK_VOID);
 
   Token::Value tok = peek();
 
   // 'import' ModuleSpecifier ';'
   if (tok == Token::STRING) {
-    const AstRawString* module_specifier = ParseModuleSpecifier(CHECK_OK);
-    ExpectSemicolon(CHECK_OK);
+    const AstRawString* module_specifier = ParseModuleSpecifier(CHECK_OK_VOID);
+    ExpectSemicolon(CHECK_OK_VOID);
     scope_->module()->AddEmptyImport(
         module_specifier, scanner()->location(), zone());
-    return nullptr;
+    return;
   }
 
   // Parse ImportedDefaultBinding if present.
   const AstRawString* import_default_binding = nullptr;
   Scanner::Location import_default_binding_loc;
   if (tok != Token::MUL && tok != Token::LBRACE) {
     import_default_binding =
-        ParseIdentifier(kDontAllowRestrictedIdentifiers, CHECK_OK);
+        ParseIdentifier(kDontAllowRestrictedIdentifiers, CHECK_OK_VOID);
     import_default_binding_loc = scanner()->location();
-    DeclareImport(import_default_binding, pos, CHECK_OK);
+    DeclareImport(import_default_binding, pos, CHECK_OK_VOID);
   }
 
   // Parse NameSpaceImport or NamedImports if present.
   const AstRawString* module_namespace_binding = nullptr;
   Scanner::Location module_namespace_binding_loc;
   const ZoneList<const NamedImport*>* named_imports = nullptr;
   if (import_default_binding == nullptr || Check(Token::COMMA)) {
     switch (peek()) {
       case Token::MUL: {
         Consume(Token::MUL);
-        ExpectContextualKeyword(CStrVector("as"), CHECK_OK);
+        ExpectContextualKeyword(CStrVector("as"), CHECK_OK_VOID);
         module_namespace_binding =
-            ParseIdentifier(kDontAllowRestrictedIdentifiers, CHECK_OK);
+            ParseIdentifier(kDontAllowRestrictedIdentifiers, CHECK_OK_VOID);
         module_namespace_binding_loc = scanner()->location();
         break;
       }
 
       case Token::LBRACE:
-        named_imports = ParseNamedImports(pos, CHECK_OK);
+        named_imports = ParseNamedImports(pos, CHECK_OK_VOID);
         break;
 
       default:
         *ok = false;
         ReportUnexpectedToken(scanner()->current_token());
-        return nullptr;
+        return;
     }
   }
 
-  ExpectContextualKeyword(CStrVector("from"), CHECK_OK);
-  const AstRawString* module_specifier = ParseModuleSpecifier(CHECK_OK);
-  ExpectSemicolon(CHECK_OK);
+  ExpectContextualKeyword(CStrVector("from"), CHECK_OK_VOID);
+  const AstRawString* module_specifier = ParseModuleSpecifier(CHECK_OK_VOID);
+  ExpectSemicolon(CHECK_OK_VOID);
 
   // Now that we have all the information, we can make the appropriate
   // declarations.
 
   if (module_namespace_binding != nullptr) {
     scope_->module()->AddStarImport(
         module_namespace_binding, module_specifier,
         module_namespace_binding_loc, zone());
     // TODO(neis): Create special immutable binding for the namespace object.
   }
 
   // TODO(neis): Would prefer to call DeclareImport below rather than above and
   // in ParseNamedImports, but then a possible error message would point to the
   // wrong location.  Maybe have a DeclareAt version of Declare that takes a
   // location?
 
   if (import_default_binding != nullptr) {
     scope_->module()->AddImport(
         ast_value_factory()->default_string(), import_default_binding,
         module_specifier, import_default_binding_loc, zone());
-    // DeclareImport(import_default_binding, pos, CHECK_OK);
+    // DeclareImport(import_default_binding, pos, CHECK_OK_VOID);
   }
 
   if (named_imports != nullptr) {
     if (named_imports->length() == 0) {
       scope_->module()->AddEmptyImport(
           module_specifier, scanner()->location(), zone());
     } else {
       for (int i = 0; i < named_imports->length(); ++i) {
         const NamedImport* import = named_imports->at(i);
         scope_->module()->AddImport(
             import->import_name, import->local_name,
             module_specifier, import->location, zone());
-        // DeclareImport(import->local_name, pos, CHECK_OK);
+        // DeclareImport(import->local_name, pos, CHECK_OK_VOID);
       }
     }
   }
-
-  return nullptr;
 }
 
 
 Statement* Parser::ParseExportDefault(bool* ok) {
   //  Supports the following productions, starting after the 'default' token:
   //    'export' 'default' HoistableDeclaration
   //    'export' 'default' ClassDeclaration
   //    'export' 'default' AssignmentExpression[In] ';'
 
   Expect(Token::DEFAULT, CHECK_OK);
@@ skipping 316 matching lines @@
   // scope.
   // Let/const variables are always added to the immediately enclosing scope.
   Scope* scope =
       IsLexicalVariableMode(mode) ? scope_ : scope_->DeclarationScope();
   return scope->NewUnresolved(factory(), name, Variable::NORMAL,
                               scanner()->location().beg_pos,
                               scanner()->location().end_pos);
 }
 
 
-void* Parser::DeclareImport(const AstRawString* local_name, int pos, bool* ok) {
+void Parser::DeclareImport(const AstRawString* local_name, int pos, bool* ok) {
   DCHECK_NOT_NULL(local_name);
   VariableProxy* proxy = NewUnresolved(local_name, IMPORT);
   Declaration* declaration =
       factory()->NewVariableDeclaration(proxy, IMPORT, scope_, pos);
-  Declare(declaration, DeclarationDescriptor::NORMAL, true, CHECK_OK);
-  return nullptr;
+  Declare(declaration, DeclarationDescriptor::NORMAL, true, CHECK_OK_VOID);
 }
 
 
 Variable* Parser::Declare(Declaration* declaration,
                           DeclarationDescriptor::Kind declaration_kind,
                           bool resolve, bool* ok, Scope* scope) {
   VariableProxy* proxy = declaration->proxy();
   DCHECK(proxy->raw_name() != NULL);
   const AstRawString* name = proxy->raw_name();
   VariableMode mode = declaration->mode();
@@ skipping 2151 matching lines @@
   //
   if (expr->IsBinaryOperation()) {
     BinaryOperation* binop = expr->AsBinaryOperation();
     // The classifier has already run, so we know that the expression is a valid
     // arrow function formals production.
     DCHECK_EQ(binop->op(), Token::COMMA);
     Expression* left = binop->left();
     Expression* right = binop->right();
     int comma_pos = binop->position();
     ParseArrowFunctionFormalParameters(parameters, left, comma_pos,
-                                       CHECK_OK_CUSTOM(Void));
+                                       CHECK_OK_VOID);
     // LHS of comma expression should be unparenthesized.
     expr = right;
   }
 
   // Only the right-most expression may be a rest parameter.
   DCHECK(!parameters->has_rest);
 
   bool is_rest = expr->IsSpread();
   if (is_rest) {
     expr = expr->AsSpread()->expression();
@@ skipping 58 matching lines @@
   body->Add(factory()->NewExpressionStatement(init_generator_variable,
                                               kNoSourcePosition),
             zone());
 
   Block* try_block = factory()->NewBlock(NULL, 8, true, kNoSourcePosition);
 
   ZoneList<Statement*>* inner_body = try_block->statements();
 
   Expression* return_value = nullptr;
   if (body_type == FunctionBody::Normal) {
-    ParseStatementList(inner_body, Token::RBRACE, CHECK_OK_CUSTOM(Void));
+    ParseStatementList(inner_body, Token::RBRACE, CHECK_OK_VOID);
     return_value = factory()->NewUndefinedLiteral(kNoSourcePosition);
   } else {
     return_value =
-        ParseAssignmentExpression(accept_IN, classifier, CHECK_OK_CUSTOM(Void));
-    ParserTraits::RewriteNonPattern(classifier, CHECK_OK_CUSTOM(Void));
+        ParseAssignmentExpression(accept_IN, classifier, CHECK_OK_VOID);
+    ParserTraits::RewriteNonPattern(classifier, CHECK_OK_VOID);
   }
 
   return_value = BuildPromiseResolve(return_value, return_value->position());
   inner_body->Add(
       factory()->NewReturnStatement(return_value, return_value->position()),
       zone());
   body->Add(BuildRejectPromiseOnException(try_block), zone());
   scope->set_end_position(scanner()->location().end_pos);
 }
 
@@ skipping 16 matching lines @@
 }
 
 
 void ParserTraits::ParseArrowFunctionFormalParameterList(
     ParserFormalParameters* parameters, Expression* expr,
     const Scanner::Location& params_loc,
     Scanner::Location* duplicate_loc, bool* ok) {
   if (expr->IsEmptyParentheses()) return;
 
   ParseArrowFunctionFormalParameters(parameters, expr, params_loc.end_pos,
-                                     CHECK_OK_CUSTOM(Void));
+                                     CHECK_OK_VOID);
 
   if (parameters->Arity() > Code::kMaxArguments) {
     ReportMessageAt(params_loc, MessageTemplate::kMalformedArrowFunParamList);
     *ok = false;
     return;
   }
 
   Type::ExpressionClassifier classifier(parser_);
   if (!parameters->is_simple) {
     classifier.RecordNonSimpleParameter();
@@ skipping 303 matching lines @@
     // data contains the information we need to construct the lazy function.
     FunctionEntry entry =
         cached_parse_data_->GetFunctionEntry(function_block_pos);
     // Check that cached data is valid. If not, mark it as invalid (the embedder
     // handles it). Note that end position greater than end of stream is safe,
     // and hard to check.
     if (entry.is_valid() && entry.end_pos() > function_block_pos) {
       scanner()->SeekForward(entry.end_pos() - 1);
 
       scope_->set_end_position(entry.end_pos());
-      Expect(Token::RBRACE, CHECK_OK_CUSTOM(Void));
+      Expect(Token::RBRACE, CHECK_OK_VOID);
       total_preparse_skipped_ += scope_->end_position() - function_block_pos;
       *materialized_literal_count = entry.literal_count();
       *expected_property_count = entry.property_count();
       SetLanguageMode(scope_, entry.language_mode());
       if (entry.uses_super_property()) scope_->RecordSuperPropertyUsage();
       if (entry.calls_eval()) scope_->RecordEvalCall();
       return;
     }
     cached_parse_data_->Reject();
   }
@@ skipping 12 matching lines @@
     return;
   }
   if (logger.has_error()) {
     ParserTraits::ReportMessageAt(
         Scanner::Location(logger.start(), logger.end()), logger.message(),
         logger.argument_opt(), logger.error_type());
     *ok = false;
     return;
   }
   scope_->set_end_position(logger.end());
-  Expect(Token::RBRACE, CHECK_OK_CUSTOM(Void));
+  Expect(Token::RBRACE, CHECK_OK_VOID);
   total_preparse_skipped_ += scope_->end_position() - function_block_pos;
   *materialized_literal_count = logger.literals();
   *expected_property_count = logger.properties();
   SetLanguageMode(scope_, logger.language_mode());
   if (logger.uses_super_property()) {
     scope_->RecordSuperPropertyUsage();
   }
   if (logger.calls_eval()) {
     scope_->RecordEvalCall();
   }
@@ skipping 1268 matching lines @@
     if (property == nullptr) return;
     // Do not rewrite (computed) key expressions
     AST_REWRITE_PROPERTY(Expression, property, value);
   }
 
   Parser* parser_;
 };
 
 
 void Parser::RewriteNonPattern(ExpressionClassifier* classifier, bool* ok) {
-  ValidateExpression(classifier, CHECK_OK_CUSTOM(Void));
+  ValidateExpression(classifier, CHECK_OK_VOID);
   auto non_patterns_to_rewrite = function_state_->non_patterns_to_rewrite();
   int begin = classifier->GetNonPatternBegin();
   int end = non_patterns_to_rewrite->length();
   if (begin < end) {
     NonPatternRewriter rewriter(stack_limit_, this);
     for (int i = begin; i < end; i++) {
       DCHECK(non_patterns_to_rewrite->at(i)->IsRewritableExpression());
       rewriter.Rewrite(non_patterns_to_rewrite->at(i));
     }
     non_patterns_to_rewrite->Rewind(begin);
@@ skipping 1181 matching lines @@
 }
 
 #ifdef DEBUG
 void Parser::Print(AstNode* node) {
   ast_value_factory()->Internalize(Isolate::Current());
   node->Print(Isolate::Current());
 }
 #endif  // DEBUG
 
 #undef CHECK_OK
-#undef CHECK_OK_CUSTOM
+#undef CHECK_OK_VOID
 #undef CHECK_FAILED
 
 }  // namespace internal
 }  // namespace v8