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,
On 2015/06/15 22:01:09, noordhuis wrote:
TARGET_RESTRICTED doesn't seem to be used anywhere.
Done.
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;
On 2015/06/15 22:01:09, noordhuis wrote:
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.
Done.
https://codereview.chromium.org/1168643005/diff/300001/src/expression-classifier.h#newcode142
src/expression-classifier.h:142: return lhs_type_ == TARGET_PATTERN &&
assigned_;
On 2015/06/15 22:01:09, noordhuis wrote:
I think this can be written as `return IsValidPattern() &&
IsAssigned()`?
Done.
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 =
On 2015/06/15 22:01:09, noordhuis wrote:
Maybe make this `const bool`. The mutable version immediately made me
scan the
code below for reassignment that doesn't happen.
Done.
https://codereview.chromium.org/1168643005/diff/300001/src/preparser.h#newcode2924
src/preparser.h:2924: classifier->set_assigned();
On 2015/06/15 22:01:09, noordhuis wrote:
This is a little easier to comprehend if you swap the clauses around
(although
it makes the diff noisier.)
Done.
https://codereview.chromium.org/1168643005/diff/300001/test/cctest/test-parsing.cc
File test/cctest/test-parsing.cc (right):
https://codereview.chromium.org/1168643005/diff/300001/test/cctest/test-parsing.cc#newcode6565
test/cctest/test-parsing.cc:6565: "{ x : y }",
On 2015/06/19 08:57:30, Dmitry S. Lomov wrote:
Add more tests of the form:
{ x : y.z } = ...
{ x : foo().x ) = ...
and so on (where the binding location is not a valid binding)
You have some of these, but not all. Consider the tests of "pattern
context":
{ x : E }
{ x : { y : E }}
[E]
[a, b, E]
....
where E is various valid assignment targets (x.y, foo(y), x[i] etc.)
I think there are cases for each of those, but I've added a bunch more
just to be extra systematic about it
https://codereview.chromium.org/1168643005/
--
--
v8-dev mailing list
v8-dev@googlegroups.com
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 v8-dev+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
