Thank you, Kevin. I incorporated your feedback. Please take another look.
Thanks!
Martin
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);
On 2011/01/21 11:41:56, Kevin Millikin wrote:
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();
Done.
http://codereview.chromium.org/6382006/diff/1/src/parser.cc#newcode3307
src/parser.cc:3307: if (name_loc.beg_pos == RelocInfo::kNoPosition &&
On 2011/01/21 11:41:56, Kevin Millikin wrote:
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().
Done. The Location::IsValid felt best so I went with that one.
The only slight glitch is that the scanner.h doesn't see assembler.h
where kNoPosition is located so the factory simply uses -1 literal.
http://codereview.chromium.org/6382006/diff/1/src/parser.cc#newcode3309
src/parser.cc:3309: // Store location for later
On 2011/01/21 11:41:56, Kevin Millikin wrote:
This comment is probably redundant and so's the one in the body of the
next if.
Done.
http://codereview.chromium.org/6382006/diff/1/src/parser.cc#newcode3313
src/parser.cc:3313: top_scope_->IsDeclared(param_name)) {
On 2011/01/21 11:41:56, Kevin Millikin wrote:
Indentation is off here.
Done.
http://codereview.chromium.org/6382006/diff/1/src/parser.cc#newcode3395
src/parser.cc:3395: ? function_token_position
On 2011/01/21 11:41:56, Kevin Millikin wrote:
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 ....
Done.
http://codereview.chromium.org/6382006/
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
