LGTM. Superficial comments below.
http://codereview.chromium.org/6382006/diff/1/src/parser.cc File src/parser.cc (right): http://codereview.chromium.org/6382006/diff/1/src/parser.cc#newcode3299 src/parser.cc:3299: Scanner::Location name_loc(RelocInfo::kNoPosition, RelocInfo::kNoPosition); This might be more readable with a factory function for "no location" in the scanner class so you can write: Scanner::Location name_loc = Scanner::NoLocation(); http://codereview.chromium.org/6382006/diff/1/src/parser.cc#newcode3307 src/parser.cc:3307: if (name_loc.beg_pos == RelocInfo::kNoPosition && Likewise, struct Scanner::Location could have a predicate for whether the location is set so you could write: if (name_loc.HasLocation() ... If it seems to strange to ask a location if it has a location, we could try something like Scanner::InvalidLocation() and Location::IsValid(). http://codereview.chromium.org/6382006/diff/1/src/parser.cc#newcode3309 src/parser.cc:3309: // Store location for later This comment is probably redundant and so's the one in the body of the next if. http://codereview.chromium.org/6382006/diff/1/src/parser.cc#newcode3313 src/parser.cc:3313: top_scope_->IsDeclared(param_name)) { Indentation is off here. http://codereview.chromium.org/6382006/diff/1/src/parser.cc#newcode3395 src/parser.cc:3395: ? function_token_position We usually indent these continuations 4 spaces from the line above so save them running away to the right: int position = function_token_position != RelocInfo::kNoPosition ? function_token_position .... http://codereview.chromium.org/6382006/ -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
