Sorry Caitlin, nothing insightful here, just some nits. The general
approach
looks alright to me.
https://codereview.chromium.org/1168643005/diff/300001/src/expression-classifier.h
File src/expression-classifier.h (right):
https://codereview.chromium.org/1168643005/diff/300001/src/expression-classifier.h#newcode91
src/expression-classifier.h:91: TARGET_RESTRICTED,
TARGET_RESTRICTED doesn't seem to be used anywhere.
https://codereview.chromium.org/1168643005/diff/300001/src/expression-classifier.h#newcode126
src/expression-classifier.h:126: return lhs_type_ >= TARGET_IDENTIFIER
&& lhs_type_ <= TARGET_PROPERTY;
This code is perhaps a little easier to understand if you use == instead
of <= and >= because then you don't have to look up the enum and check
what falls in that range.
https://codereview.chromium.org/1168643005/diff/300001/src/expression-classifier.h#newcode142
src/expression-classifier.h:142: return lhs_type_ == TARGET_PATTERN &&
assigned_;
I think this can be written as `return IsValidPattern() &&
IsAssigned()`?
https://codereview.chromium.org/1168643005/diff/300001/src/preparser.h
File src/preparser.h (right):
https://codereview.chromium.org/1168643005/diff/300001/src/preparser.h#newcode514
src/preparser.h:514: bool rewrite =
Maybe make this `const bool`. The mutable version immediately made me
scan the code below for reassignment that doesn't happen.
https://codereview.chromium.org/1168643005/diff/300001/src/preparser.h#newcode2924
src/preparser.h:2924: classifier->set_assigned();
This is a little easier to comprehend if you swap the clauses around
(although it makes the diff noisier.)
https://codereview.chromium.org/1168643005/
--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/d/optout.
