On 2015/12/09 13:52:08, rossberg wrote:
FWIW, Niko is looking into simplifying the rewriting mechanism as part of
his
patch now.
Sounds good. Can you add me to the patch (when it gets sent out, that is)?
https://codereview.chromium.org/1309813007/
--
--
v8-dev mailing list
I actually think this is looking pretty good. Why do you think this is
fragile
(I mean, it at least doesn't seem any more fragile than the last two
iterations)?
My one big request is to de-generalize RewritableExpression (see comments in
ast.h and elsewhere).
ion::kDestructuringAssignment)) {
On 2015/12/01 22:51:00, caitp wrote:
On 2015/12/01 22:42:56, adamk wrote:
> It's checks like this that I don't see as terribly useful, since
they're
always
> true.
the check is checking 2 things: 1, that it's "possibly" a
destructuring
assignment (eg, v
On 2015/12/02 00:20:51, caitp wrote:
Alright, re-fixed
Fixed is good, ping for re-review when you've de-generalized
RewritableExpression (or when you make an argument for keeping it as-is).
https://codereview.chromium.org/1309813007/
--
--
v8-dev mailing list
v8-dev@googlegroups.com
I've noted what I mean by de-generalizing.
I'm all for reusable code, but I tend to lean towards doing that once there
are
multiple cases. This ensures that the general code actually serves multiple
use-cases well. As-is, this code looks general but in fact only handles a
single
case.
lgtm once nits are taken care of and the CL description is trimmed
down/rewritten to match reality (for one, this is much more than a parsing
patch
now).
Will also need some OWNERS in Munich, adding bmeurer who's in all the
relevant
files.
A few nits below.
On 2015/11/30 15:47:37, caitp wrote:
So, I'm still not really happy with this approach, but it is what it is.
Maybe there's a better way to do this, like just introducing a new AST
node
like
"RewritableExpression" or something, and wrapping the original Assignment
in
that if it will be
On 2015/11/30 19:20:28, adamk wrote:
On 2015/11/30 15:47:37, caitp wrote:
> So, I'm still not really happy with this approach, but it is what it is.
>
> Maybe there's a better way to do this, like just introducing a new AST
node
like
> "RewritableExpression" or s
:
On 2015/11/25 21:05:28, adamk wrote:
> Can you use an RAII object for this instead of relying on
set_context at the
end
> of the block to handle this?
Doable, but it is (relatively) a lot of code and I don't think it
needs to be
used a whole lot. I'll put that in the next patch
I understand more and more each time, quite exciting.
https://codereview.chromium.org/1309813007/diff/470001/src/ast-expression-visitor.cc
File src/ast-expression-visitor.cc (right):
https://codereview.chromium.org/1309813007/diff/470001/src/ast-expression-visitor.cc#newcode270
Overall I really do prefer putting this stuff in Assignment. A few questions
about other additions, but I think this is on the right track.
https://codereview.chromium.org/1309813007/diff/410001/src/expression-classifier.h
File src/expression-classifier.h (right):
On 2015/11/20 19:56:45, caitp wrote:
On 2015/11/20 19:49:48, adamk wrote:
> Having read through a few times, I have some high-level questions:
>
> - You mentioned on IRC that the keeping-track-of-assigments-to-rewrite
thing
> didn't interact well with something else. Was that resol
Having read through a few times, I have some high-level questions:
- You mentioned on IRC that the keeping-track-of-assigments-to-rewrite thing
didn't interact well with something else. Was that resolved? Overall, do you
know of any cases where this doesn't work correctly?
- It seems a bit
Another source of tests for this would be the test262 tests in
language/expressions/assignment/destructuring (currently all skipped).
Here are some comments on the current patch. This looks like a good
direction to
me; I'd really like to see stuff moved up into Assignment, though (I've
noted
ction")
On 2015/11/20 23:10:25, caitp wrote:
On 2015/11/20 22:42:58, adamk wrote:
> Looks like a stray change here
It's from a rebase, so not everything in the inter diff is mine
This shows up in the whole diff, not the interdiff.
https://codereview.chromium.org/1309813007/
--
--
v
It seems to me that supporting parsing of destructuring assignment and
improving
error messages for destructuring parse failures are separable tasks. I'd
find
patches for the former easier to read if they didn't also make changes to
the
latter at the same time.
:11:41, adamk wrote:
> Why is this not directly calling RecordAssignmentPatternError? Were
we getting
> this wrong before, or does this have to do with the ambiguity in
arrow params?
>
> Same question for the below calls to RecordPatternError.
It's an invalid binding pattern as
Understanding more and more (or so I think)...
https://codereview.chromium.org/1309813007/diff/210001/src/expression-classifier.h
File src/expression-classifier.h (right):
https://codereview.chromium.org/1309813007/diff/210001/src/expression-classifier.h#newcode319
Back in the USA, and now I've lost state on this. Is this the change I
ought to
be reviewing? Or was there more cleanup you wanted to do on it before
continuing?
https://codereview.chromium.org/1309813007/
--
--
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
:
On 2015/10/27 23:04:18, adamk wrote:
> Can't you wrap the next few lines in a block and use an RAII object
here
instead
> of calling these directly?
As mentioned, the second version gets rid of all this extra parser
state, it may
be a simpler. Which one do you think would be pre
It's taking me longer than I'd like to boot up on this (hard to keep the
whole
thing in my head), but here's some feedback in the meantime (while I read
and
re-read).
https://codereview.chromium.org/1309813007/diff/150001/src/ast.h
File src/ast.h (right):
On 2015/08/04 09:47:36, rossberg wrote:
https://codereview.chromium.org/1260053004/diff/20001/src/parser.cc
File src/parser.cc (right):
https://codereview.chromium.org/1260053004/diff/20001/src/parser.cc#newcode4436
src/parser.cc:4436: VariableMode fvar_mode = use_strict_const ? CONST :
On 2015/10/09 22:24:28, adamk wrote:
On 2015/08/04 09:47:36, rossberg wrote:
> https://codereview.chromium.org/1260053004/diff/20001/src/parser.cc
> File src/parser.cc (right):
>
>
https://codereview.chromium.org/1260053004/diff/20001/src/parser.cc#newcode4436
> src
lgtm with an updated CL description noting that this only changes behavior
for
@@isConcatSpreadable for now.
https://codereview.chromium.org/1230793002/
--
--
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
---
You received this message because you are
I'm happy that Toon is happy with this approach. I'm still a bit worried
about
the "not yet specced" part, though, especially as we're adding this new
behavior
to existing Symbols. Is there a reason not to start this with only allowing
window[@@isConcatSpreadable] to return undefined?
On 2015/07/18 07:59:19, Toon Verwaest wrote:
What about setting a bit on symbols whether they are "absent on access
check
failure", and extend GetProperty(Attributes)WithFailedAccessCheck to also
handle
such symbols in addition to all_can_read accessors and interceptors? That
way
it
On 2015/09/29 21:48:28, adamk wrote:
On 2015/07/18 07:59:19, Toon Verwaest wrote:
> What about setting a bit on symbols whether they are "absent on access
check
> failure", and extend GetProperty(Attributes)WithFailedAccessCheck to
also
handle
> such symbols in addit
On 2015/09/22 15:33:05, rossberg wrote:
https://codereview.chromium.org/1317033005/diff/20001/src/preparser.h
File src/preparser.h (right):
https://codereview.chromium.org/1317033005/diff/20001/src/preparser.h#newcode3288
src/preparser.h:3288:
This is not a sufficient fix, see attached bug for details.
https://codereview.chromium.org/1317033005/
--
--
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
Reviewers: rossberg, wingo, caitp,
Message:
I'm not at all sure we want to land this, but I wanted to bump its
visibility up
a bit.
Description:
Always lazy compile arrow functions
This causes the scope of default parameters to be correct, where otherwise
we'd get it wrong, as expressions
:10, Dan Ehrenberg wrote:
On 2015/09/11 at 15:42:56, adamk wrote:
> I'm a little bit confused: why do you use the passed-in scope above,
but the
current scope_ here (since you don't pass the last argument)? Are
|scope| and
|scope_| actually the same here?
It happens to work beca
+bmeurer for various compiler OWNERs
https://codereview.chromium.org/1332873003/
--
--
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
lgtm % a C++11 nit
https://codereview.chromium.org/1332873003/diff/160001/src/parser.cc
File src/parser.cc (right):
https://codereview.chromium.org/1332873003/diff/160001/src/parser.cc#newcode4974
src/parser.cc:4974: for (auto delegate : *delegates) {
With the renaming, I think this would be
lgtm
Sorry for the delay, I was expecting a ping from your end when you uploaded
a
new patch.
https://codereview.chromium.org/1328083002/
--
--
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
First round of comments, mostly style nits. The juicy bits are the
handling of
scopes in the fixup code, and some suggestions for tests to exercise
interaction
with other fancy scoping features.
https://codereview.chromium.org/1332873003/diff/80001/src/ast.h
File src/ast.h (right):
I haven't looked at the patch yet (just back from vacation), but I don't
think
we can switch from %AppendElement to simple assignment. V8 should match the
spec, not Babel.
What do you mean by "wasn't really correct before"?
https://codereview.chromium.org/1308123007/
--
--
v8-dev mailing
On 2015/09/08 20:46:41, caitp wrote:
On 2015/09/08 20:41:36, caitp wrote:
> On 2015/09/08 20:23:54, adamk wrote:
> > I haven't looked at the patch yet (just back from vacation), but I
don't
think
> > we can switch from %AppendElement to simple assignment. V8 should
match
lgtm
https://codereview.chromium.org/1317113007/
--
--
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
I agree that some refactoring here would be helpful for readability, but
I'm not
sure the switch statement you landed on was it. So I'd suggest going with a
minimal fix and dealing with the refactoring separately.
https://codereview.chromium.org/1328083002/diff/40001/src/preparser.h
File
Another high-level comment below. Performance aside, we should try to get
the
behavior correct.
https://codereview.chromium.org/1308123007/diff/1/test/mjsunit/harmony/destructuring.js
File test/mjsunit/harmony/destructuring.js (right):
://codereview.chromium.org/1308123007/diff/1/test/mjsunit/harmony/destructuring.js#newcode959
test/mjsunit/harmony/destructuring.js:959: assertDoesNotThrow("function
f({x})
{
var x; }; f({});");
On 2015/09/09 00:07:02, adamk wrote:
> So this is no longer an error, but yet
>
> function f({x =
lgtm
https://codereview.chromium.org/1321013005/
--
--
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
Given that the new behavior matches FF, this lgtm. For posterity, here's why
those results change (this is the current behavior in FF, and the new
behavior
in Chrome):
typeof document.createElement('object')
"function"
typeof document.createElement('embed')
"function"
typeof
https://codereview.chromium.org/1316233005/diff/1/BUILD.gn
File BUILD.gn (right):
https://codereview.chromium.org/1316233005/diff/1/BUILD.gn#newcode28
BUILD.gn:28: if (is_msan) {
Why is this arm64 only? My gyp-fu is weak, but I don't see a similar
stanza in the gyp files.
I'd appreciate a
Thanks, lgtm. The CQ should be able to land for you, let's give it a shot.
https://codereview.chromium.org/1316233005/
--
--
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"
https://codereview.chromium.org/1272673003/diff/150001/src/preparser.h
File src/preparser.h (right):
https://codereview.chromium.org/1272673003/diff/150001/src/preparser.h#newcode34
src/preparser.h:34: mutable int rest_array_literal_index = -1;
On 2015/09/01 15:17:24, caitp wrote:
On
lgtm
https://codereview.chromium.org/1327483002/
--
--
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
lgtm, thanks for the fix (and sorry for the breakage). Python is just not my
thing...
https://codereview.chromium.org/1298973007/
--
--
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
On 2015/08/31 11:54:08, Michael Achenbach wrote:
lgtm - note that this string is just used for printing in the end. It
makes no
functional difference.
Yup, I understand. I noticed when I copied and pasted the commits to double
check with git show.
https://codereview.chromium.org/1298973007/
lgtm
https://codereview.chromium.org/1317403002/
--
--
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
This is looking good to me, will look once more after you've handled the
rest of
wingo's comments.
I agree that we should do something nicer with the literals count, it's
quite
messy at the moment and not at all obvious why the count is valid in all
cases.
+bmeurer for full-codegen
lgtm % one more wingo comment I point to
https://codereview.chromium.org/1272673003/diff/110001/src/parser.cc
File src/parser.cc (right):
https://codereview.chromium.org/1272673003/diff/110001/src/parser.cc#newcode4414
src/parser.cc:4414: if (parameter.is_rest) {
On 2015/08/31 13:07:31, wingo
lgtm, but CL description maybe should note that this is a test-only change
https://codereview.chromium.org/1318253002/
--
--
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
I will take a look at this today (sorry I've been slow to respond this week,
have been swamped with non-engineering tasks)
https://codereview.chromium.org/1272673003/
--
--
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
---
You received this message because
Excited to see this passing tests!
As you suspected, I'm a little bit skeptical of adding a new AST node for
RestParameter. I'm probably missing something, but it seems like a big
hammer
just to make reindexing work, as no other AST visitor will ever care about
it.
Not sure who else would
lgtm, though as discussed offline please add a test case to the regression
test
for the undefined case.
https://codereview.chromium.org/1309303006/diff/20001/src/parser.cc
File src/parser.cc (right):
https://codereview.chromium.org/1309303006/diff/20001/src/parser.cc#newcode2993
rubberstamp lgtm as this follows similar patches elsewhere in Chromium
https://codereview.chromium.org/1295673002/
--
--
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.
I'd appreciate if someone from the Language team was included on changes
like
this. There's some behavior change in this patch that may cause problems on
the
web, see below.
https://codereview.chromium.org/1306303003/diff/20001/src/objects.cc
File src/objects.cc (right):
:37, adamk wrote:
So is const-correctness the only thing keeping you from handling
rest
specially
here, instead of adding a new AST node? The AST node adds a good bit
of
cognitive overhead, and a very strange new type of Expression
(though I
guess
ExpressionClassifier already let that boat
Your reasoning re: ScopeInfo sounds right to me. Can you add a note to
scopes.h
explaining why the bit isn't needed on ScopeInfo?
Also, could you add a few more tests (the ones suggested by Andreas)? In
particular, tests for assignment and loading inside evals (both sloppy and
strict would be
lgtm
https://codereview.chromium.org/1320673007/
--
--
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
lgtm once you've added sloppy eval tests
https://codereview.chromium.org/1312613003/
--
--
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
Final list of things that need to be fixed before landing:
- Renaming of NeedsHoleCheck method
- Test for sloppy eval
Also on the menu, but can be handled in a follow-up (since it's not related
to
hole-checking): add tests for completion value of switch, and fix the
desugaring
to handle
lgtm, but would like to see vogelheim's OK on the bookmark stuff (wonder if
there's a way to test that).
Also, could you update the CL description with the 2-3% number? Or do you
want
to wait to see if the UNLIKELY macro smooths that out?
https://codereview.chromium.org/1315673009/
--
--
lgtm, I think you've satisfied rossberg's requirements with the added tests
and
comment in scopes.h
https://codereview.chromium.org/1312613003/
--
--
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to
Thanks. Was able to add a RemoveUnresolved bandaid to fix the strong mode
failures, but what really needs to be done is an overhaul of the way scoping
works for catch to match ES6. Might work on that before tackling
destructuring.
https://codereview.chromium.org/1315483004/
--
--
v8-dev
+littledan
https://codereview.chromium.org/1307943007/
--
--
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
lgtm % nits
https://codereview.chromium.org/1312613003/diff/40001/src/full-codegen/full-codegen.cc
File src/full-codegen/full-codegen.cc (right):
https://codereview.chromium.org/1312613003/diff/40001/src/full-codegen/full-codegen.cc#newcode1601
src/full-codegen/full-codegen.cc:1601:
lgtm
https://codereview.chromium.org/1311163002/
--
--
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
https://codereview.chromium.org/1314493005/diff/20001/src/runtime.js
File src/runtime.js (right):
https://codereview.chromium.org/1314493005/diff/20001/src/runtime.js#newcode739
src/runtime.js:739: arg = %_MathFloor(arg);
This line is the problem. I get a crash if |arg| is a Smi. Repro:
d8
lgtm % nits if this is the approach we want to take, but please wait for the
go-ahead from amineer/seththompson before landing/merging. I hope we can
avoid
landing this.
https://codereview.chromium.org/1308713005/diff/1/test/mjsunit/mjsunit.status
File test/mjsunit/mjsunit.status (right):
and re-adding rossberg, the race is on!
https://codereview.chromium.org/1307943007/
--
--
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
lgtm
https://codereview.chromium.org/1316773004/
--
--
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
lgtm
Looks like this was a lot easier than last time!
https://codereview.chromium.org/1317723003/
--
--
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
On 2015/08/25 04:53:48, Benedikt Meurer wrote:
Hey Adam,
I'm currently working on implementing ToPrimitive for ES6, in a way that
we
can
have %_ToPrimitive, %_ToNumber and %_ToString (with proper code stubs and
also
proper support for those within the runtime without resorting to
lgtm too
https://codereview.chromium.org/1314543005/
--
--
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
https://codereview.chromium.org/1307223002/diff/40001/src/ast.h
File src/ast.h (right):
https://codereview.chromium.org/1307223002/diff/40001/src/ast.h#newcode1493
src/ast.h:1493: return boilerplate_properties_ == 0
properties_-length() 0;
I don't think the LHS of this expression is correct:
https://codereview.chromium.org/1311163002/diff/1/src/scopes.h
File src/scopes.h (right):
https://codereview.chromium.org/1311163002/diff/1/src/scopes.h#newcode133
src/scopes.h:133: bool is_optional, bool is_rest, bool* is_duplicate);
Two bools in a row suggests to me that an enum would be
lgtm
https://codereview.chromium.org/1309163003/
--
--
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
https://codereview.chromium.org/1307223002/diff/40001/src/ast.h
File src/ast.h (right):
https://codereview.chromium.org/1307223002/diff/40001/src/ast.h#newcode1493
src/ast.h:1493: return boilerplate_properties_ == 0
properties_-length() 0;
On 2015/08/24 18:00:53, adamk wrote:
I don't think
https://codereview.chromium.org/1307223002/diff/40001/src/ast.h
File src/ast.h (right):
https://codereview.chromium.org/1307223002/diff/40001/src/ast.h#newcode1493
src/ast.h:1493: return boilerplate_properties_ == 0
properties_-length() 0;
On 2015/08/24 18:14:29, adamk wrote:
On 2015/08/24
On 2015/08/24 18:29:50, caitp wrote:
On 2015/08/24 18:17:17, caitp wrote:
On 2015/08/24 18:14:29, adamk wrote:
https://codereview.chromium.org/1307223002/diff/40001/src/ast.h
File src/ast.h (right):
https://codereview.chromium.org/1307223002/diff/40001/src/ast.h#newcode1493
src
Ah, thanks. I think I could actually land now (since I'm an owner), but
I'll let
Andreas take a look tomorrow.
https://codereview.chromium.org/1307943007/
--
--
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
---
You received this message because you are
Does it look good enough to say the magic words? Rietveld is pretty picky :)
https://codereview.chromium.org/1307943007/
--
--
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
Reviewers: caitp,
Description:
[es6] Fix computed property names in nested literals
Make ObjectLiteral::is_simple() false for literals containing computed
property names, which causes IsCompileTimeValue() to return false and
thus force code to be generated for setting up such properties. This
https://codereview.chromium.org/1312613003/diff/20001/src/full-codegen/arm/full-codegen-arm.cc
File src/full-codegen/arm/full-codegen-arm.cc (right):
https://codereview.chromium.org/1312613003/diff/20001/src/full-codegen/arm/full-codegen-arm.cc#newcode1471
Reviewers: Benedikt Meurer,
Message:
bmeurer, this might be up your alley. CC littledan as he ran into perf
issues
with ToLength previously.
https://codereview.chromium.org/1314493005/diff/1/src/runtime.js
File src/runtime.js (right):
Code looks fine, some questions about the test.
https://codereview.chromium.org/1303033003/diff/20001/test/mjsunit/regress/regress-520029.js
File test/mjsunit/regress/regress-520029.js (right):
lgtm
https://codereview.chromium.org/1303033003/
--
--
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
lgtm
https://codereview.chromium.org/1293283002/
--
--
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
We've branched for M46, feel free to land this at your leisure.
https://codereview.chromium.org/1295543003/
--
--
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
https://codereview.chromium.org/1293283002/diff/11/src/parser.cc
File src/parser.cc (right):
https://codereview.chromium.org/1293283002/diff/11/src/parser.cc#newcode2982
src/parser.cc:2982: factory()-NewBlock(labels, 2, true,
RelocInfo::kNoPosition);
Seems weird that you pass labels
https://codereview.chromium.org/1247243003/diff/20001/src/builtins.cc
File src/builtins.cc (right):
https://codereview.chromium.org/1247243003/diff/20001/src/builtins.cc#newcode951
src/builtins.cc:951: fallback = !IsConcatSpreadable(isolate, obj);
As the test failures show, you can't make this
Reviewers: Jakob,
Description:
[api] Relax CHECK for ArrayBuffer API abuse
Zero-length ArrayBuffers are allowed to have NULL backing stores.
BUG=522496
LOG=n
Please review this at https://codereview.chromium.org/1302803003/
Base URL: https://chromium.googlesource.com/v8/v8.git@master
Haven't dug in yet, but to your issue with 'let []': I suspect you're just
testing in sloppy mode. Here are my results with this patched in:
$ d8 --harmony-destructuring -e 'let [x] = [42]; print(x)'
unnamed:1: ReferenceError: let is not defined
let [x] = [1]; print(x)
^
ReferenceError: let is
lgtm!
https://codereview.chromium.org/1237583005/
--
--
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
https://codereview.chromium.org/1295883002/diff/60001/src/scanner.h
File src/scanner.h (right):
https://codereview.chromium.org/1295883002/diff/60001/src/scanner.h#newcode547
src/scanner.h:547: (current_.literal_chars == literal_buffers_[0])
On 2015/08/15 00:28:31, adamk wrote:
Why switch
A little confused by this patch...isn't this code basically still dead? I'd
rather see these changes as part of a patch that makes use of them (which it
sounds like is forthcoming, so maybe I'll just wait for that).
https://codereview.chromium.org/1291343005/
--
--
v8-dev mailing list
cc vogelheim@, who might have some insight into evaluating performance
regressions related to CodeLoad.
https://codereview.chromium.org/1295883002/
--
--
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed
This lgtm, but let's wait until after the branch cut for M46 to land this.
https://codereview.chromium.org/1295543003/
--
--
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
1 - 100 of 1077 matches
Mail list logo
