Reviewers: adamk, rossberg,
Description:
Implement sloppy-mode block-defined functions (Annex B 3.3)
ES2015 specifies very particular semantics for functions defined in blocks.
In strict mode, it is simply a lexical binding scoped to that block. In
sloppy
mode, in addition to that lexical bin
On 2015/09/10 17:39:44, commit-bot: I haz the power wrote:
The author mailto:julien.gi...@joyent.com has not signed Google
Contributor
License
Agreement. Please visit https://cla.developers.google.com to sign and
manage
CLA.
Signed, sorry for that.
https://codereview.chromium.org/12967430
Hi Ross - I've uploaded the mips ports:
https://codereview.chromium.org/1334873002
However, there's a test failure (cctest/test-interpreter/InterpreterCall)
that I
haven't investigated deeply, since it fails on all arch's.
https://codereview.chromium.org/1323463005/
--
--
v8-dev mailing list
lgtm
Code looks good to me. I am still worried about performance though. Could
you
run some benchmarks which hit the path before committing? Either that or be
prepared to back it out later when the regression becomes apparent.
https://codereview.chromium.org/1309243003/
--
--
v8-dev mailing
CQ is trying da patch. Follow status at
https://chromium-cq-status.appspot.com/patch-status/1296743003/1
View timeline at
https://chromium-cq-status.appspot.com/patch-timeline/1296743003/1
https://codereview.chromium.org/1296743003/
--
--
v8-dev mailing list
v8-dev@googlegroups.com
http://gro
The author julien.gi...@joyent.com has not signed Google Contributor License
Agreement. Please visit https://cla.developers.google.com to sign and manage
CLA.
https://codereview.chromium.org/1296743003/
--
--
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
---
I don't know what else it might be. These kinds of crashes tend to be among
the hardest to track down. Can you catch it in GDB? Inspecting the heap
might provide valuable clues; or it might be entirely fruitless if the root
cause of whatever chain of events is going wrong has long been overwritten
Patchset 18 (id:??) landed as
https://crrev.com/8df7b4f6b502e2c318b61ce604332d51544081c6
Cr-Commit-Position: refs/heads/master@{#30687}
https://codereview.chromium.org/1291693004/
--
--
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
---
You received this messa
Committed patchset #18 (id:340001)
https://codereview.chromium.org/1291693004/
--
--
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 an
CQ is trying da patch. Follow status at
https://chromium-cq-status.appspot.com/patch-status/1291693004/340001
View timeline at
https://chromium-cq-status.appspot.com/patch-timeline/1291693004/340001
https://codereview.chromium.org/1291693004/
--
--
v8-dev mailing list
v8-dev@googlegroups.com
Thanks a lot for the reply! :-)
Alright, I figured it was worth a shot! Thanks for clarifying the
situation. The version information in our repository says:
#define V8_MAJOR_VERSION 4#define V8_MINOR_VERSION 5#define
V8_BUILD_NUMBER 103#define V8_PATCH_LEVEL 29
So it looks like we indeed have t
Sorry for the late reply; v8-dev is noisy and mails tend to get overlooked
(but we're in the process of changing that!).
I don't think the CL you've mentioned has something to do with the failure.
It deals with *double alignment*, i.e. 8-byte alignment, whereas the
failing DCHECK asserts 4-byte al
Patchset 8 (id:??) landed as
https://crrev.com/752b0308df72461bfeb644bf6dd8dd331fcdb722
Cr-Commit-Position: refs/heads/master@{#30686}
https://codereview.chromium.org/1321993004/
--
--
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
---
You received this messag
Committed patchset #8 (id:140001)
https://codereview.chromium.org/1321993004/
--
--
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
Patchset 7 (id:??) landed as
https://crrev.com/33ec0b79b8ea60dcccf1d445b0cbd2eed8e1a165
Cr-Commit-Position: refs/heads/master@{#30685}
https://codereview.chromium.org/1304923004/
--
--
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
---
You received this messag
Committed patchset #7 (id:21)
https://codereview.chromium.org/1304923004/
--
--
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
CQ is trying da patch. Follow status at
https://chromium-cq-status.appspot.com/patch-status/1304923004/21
View timeline at
https://chromium-cq-status.appspot.com/patch-timeline/1304923004/21
https://codereview.chromium.org/1304923004/
--
--
v8-dev mailing list
v8-dev@googlegroups.com
CQ is trying da patch. Follow status at
https://chromium-cq-status.appspot.com/patch-status/1321993004/140001
View timeline at
https://chromium-cq-status.appspot.com/patch-timeline/1321993004/140001
https://codereview.chromium.org/1321993004/
--
--
v8-dev mailing list
v8-dev@googlegroups.com
Thanks for the reviews, I'll port to ia32, arm and arm64 now.
+v8-mips-ports: Could you port for mips/mips64 please?
https://codereview.chromium.org/1323463005/diff/60001/src/interpreter/bytecode-array-builder.h
File src/interpreter/bytecode-array-builder.h (right):
https://codereview.chromium
lgtm. Minor spelling nit, but otherwise comprehensive.
https://codereview.chromium.org/1323463005/diff/60001/src/interpreter/bytecode-array-builder.h
File src/interpreter/bytecode-array-builder.h (right):
https://codereview.chromium.org/1323463005/diff/60001/src/interpreter/bytecode-array-build
https://codereview.chromium.org/1304923004/diff/180001/src/ast.h
File src/ast.h (right):
https://codereview.chromium.org/1304923004/diff/180001/src/ast.h#newcode3696
src/ast.h:3696: // HeapObjects which need to persist until scope
analysis must be allocated in
On 2015/09/10 14:06:36, Michael Sta
Looks great, let's land it!
https://codereview.chromium.org/1291693004/diff/260001/src/interpreter/bytecode-array-iterator.cc
File src/interpreter/bytecode-array-iterator.cc (right):
https://codereview.chromium.org/1291693004/diff/260001/src/interpreter/bytecode-array-iterator.cc#newcode67
src/
LGTM from my end.
https://codereview.chromium.org/1304923004/diff/180001/src/ast.h
File src/ast.h (right):
https://codereview.chromium.org/1304923004/diff/180001/src/ast.h#newcode3696
src/ast.h:3696: // HeapObjects which need to persist until scope
analysis must be allocated in
nit: s/HeapObjec
Looks great, let's land it!
https://codereview.chromium.org/1291693004/diff/260001/src/interpreter/bytecode-array-iterator.cc
File src/interpreter/bytecode-array-iterator.cc (right):
https://codereview.chromium.org/1291693004/diff/260001/src/interpreter/bytecode-array-iterator.cc#newcode67
src/
Thanks! All comments incorporated. Only big deltas are in
test-run-bytecode-graph-builder.cc.
https://codereview.chromium.org/1291693004/diff/260001/src/compiler/bytecode-graph-builder.cc
File src/compiler/bytecode-graph-builder.cc (right):
https://codereview.chromium.org/1291693004/diff/260001
https://codereview.chromium.org/1304923004/diff/11/src/parser.cc
File src/parser.cc (right):
https://codereview.chromium.org/1304923004/diff/11/src/parser.cc#newcode4241
src/parser.cc:4241: factory()->set_zone(outer_zone);
On 2015/09/10 12:16:54, Michael Starzinger wrote:
Suggestion as
Patchset 2 (id:??) landed as
https://crrev.com/edb30522f9fd4a1f27abbf18683239388c67ca00
Cr-Commit-Position: refs/heads/master@{#30681}
https://codereview.chromium.org/1331013003/
--
--
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
---
You received this messag
Committed patchset #2 (id:20001)
https://codereview.chromium.org/1331013003/
--
--
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
CQ is trying da patch. Follow status at
https://chromium-cq-status.appspot.com/patch-status/1331013003/20001
View timeline at
https://chromium-cq-status.appspot.com/patch-timeline/1331013003/20001
https://codereview.chromium.org/1331013003/
--
--
v8-dev mailing list
v8-dev@googlegroups.com
ht
https://codereview.chromium.org/1317113007/diff/60001/test/js-perf-test/RestParameters/rest.js
File test/js-perf-test/RestParameters/rest.js (right):
https://codereview.chromium.org/1317113007/diff/60001/test/js-perf-test/RestParameters/rest.js#newcode5
test/js-perf-test/RestParameters/rest.js:5
Patchset 5 (id:??) landed as
https://crrev.com/6da51b4b660ea5fa594c3cb13a5ac032bbe1a1fb
Cr-Commit-Position: refs/heads/master@{#30678}
https://codereview.chromium.org/1313493005/
--
--
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
---
You received this messag
Committed patchset #5 (id:80001)
https://codereview.chromium.org/1313493005/
--
--
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
Looking good, just one suggestion.
https://codereview.chromium.org/1304923004/diff/11/src/parser.cc
File src/parser.cc (right):
https://codereview.chromium.org/1304923004/diff/11/src/parser.cc#newcode4241
src/parser.cc:4241: factory()->set_zone(outer_zone);
Suggestion as discsussed offl
CQ is trying da patch. Follow status at
https://chromium-cq-status.appspot.com/patch-status/1313493005/80001
View timeline at
https://chromium-cq-status.appspot.com/patch-timeline/1313493005/80001
https://codereview.chromium.org/1313493005/
--
--
v8-dev mailing list
v8-dev@googlegroups.com
ht
All fixed, thank you!
https://codereview.chromium.org/1313493005/diff/60001/src/accessors.cc
File src/accessors.cc (right):
https://codereview.chromium.org/1313493005/diff/60001/src/accessors.cc#newcode113
src/accessors.cc:113: DescriptorArray* descriptors =
map->instance_descriptors();
On 2015
A few more minor comments. LGTM if you address those.
I've also updated the CL description to reflect the new approach.
https://codereview.chromium.org/1313493005/diff/60001/src/accessors.cc
File src/accessors.cc (right):
https://codereview.chromium.org/1313493005/diff/60001/src/accessors.cc#n
https://codereview.chromium.org/1304923004/diff/60001/src/ast.h
File src/ast.h (right):
https://codereview.chromium.org/1304923004/diff/60001/src/ast.h#newcode3280
src/ast.h:3280: : zone_(ast_value_factory->zone()),
On 2015/09/09 19:48:16, titzer wrote:
Can we rename this zone to be clearer as
lgtm, thanks!
https://codereview.chromium.org/1331013003/
--
--
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 emai
https://codereview.chromium.org/1331013003/diff/1/test/cctest/test-unscopables-hidden-prototype.cc
File test/cctest/test-unscopables-hidden-prototype.cc (right):
https://codereview.chromium.org/1331013003/diff/1/test/cctest/test-unscopables-hidden-prototype.cc#newcode62
test/cctest/test-unscopab
Looks really great. All nits except for the comment about TwoParameterTest,
but
the rest lgtm!
https://codereview.chromium.org/1291693004/diff/260001/src/compiler/bytecode-graph-builder.cc
File src/compiler/bytecode-graph-builder.cc (right):
https://codereview.chromium.org/1291693004/diff/260
Thanks for the quick review. I addressed all the comments.
I just have one question regarding the formatting here
https://codereview.chromium.org/1331013003/diff/1/test/cctest/test-unscopables-hidden-prototype.cc#newcode62
Could you please quickly guide me on this
Thanks and Regards,
Mythri
Builtin looks good modulo comment.
https://codereview.chromium.org/1323463005/diff/60001/src/x64/builtins-x64.cc
File src/x64/builtins-x64.cc (right):
https://codereview.chromium.org/1323463005/diff/60001/src/x64/builtins-x64.cc#newcode1795
src/x64/builtins-x64.cc:1795: __
Call(masm->isolate()-
And general approach looks good to me too.
https://codereview.chromium.org/1323463005/
--
--
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
can you please run tryjobs?
I kicked these for Mythri (I don't think she has permission yet).
https://codereview.chromium.org/1331013003/
--
--
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 G
overall looks good
can you please run tryjobs?
ToLocalChecked() corresponds to FromJust() which checks that the Maybe<>
value
is "just", so I'd use that where possible.
https://codereview.chromium.org/1331013003/diff/1/test/cctest/test-unscopables-hidden-prototype.cc
File test/cctest/test-u
Looks good overall, just a couple of nits.
Jochen, do you want to give it a once over too (If you are fine with the
approach I'll not bother you with future ones).
https://codereview.chromium.org/1331013003/diff/1/test/cctest/test-unscopables-hidden-prototype.cc
File test/cctest/test-unscopable
Reviewers: rmcilroy,
Message:
Please review the changes and let me know any corrections
Description:
Continuing removing deprecated functions from cctests
Removes deprecated functions from:
- test-unique.cc
- test-unscopables-hidden-prototype.cc
- test-utils-arm64.cc
- test-utils.cc
- test-vers
Try jobs failed on following builders:
v8_presubmit on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_presubmit/builds/5672)
https://codereview.chromium.org/1309243003/
--
--
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
---
CQ is trying da patch. Follow status at
https://chromium-cq-status.appspot.com/patch-status/1309243003/60001
View timeline at
https://chromium-cq-status.appspot.com/patch-timeline/1309243003/60001
https://codereview.chromium.org/1309243003/
--
--
v8-dev mailing list
v8-dev@googlegroups.com
ht
CQ is trying da patch. Follow status at
https://chromium-cq-status.appspot.com/patch-status/1309243003/60001
View timeline at
https://chromium-cq-status.appspot.com/patch-timeline/1309243003/60001
https://codereview.chromium.org/1309243003/
--
--
v8-dev mailing list
v8-dev@googlegroups.com
ht
Benedikt: I'd like to port this to the other architectures. Before I do so
could
you have a look and let me know if you are happy with the general approach?
Thanks.
https://codereview.chromium.org/1323463005/
--
--
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-d
https://codereview.chromium.org/1327033003/diff/20001/tools/run_perf.py
File tools/run_perf.py (right):
https://codereview.chromium.org/1327033003/diff/20001/tools/run_perf.py#newcode651
tools/run_perf.py:651: if profiler_run:
Suggestion: Could you maybe just use the existing extra_flags list? I
lgtm
lgmt
Looks good :)
https://codereview.chromium.org/1309243003/
--
--
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 re
53 matches
Mail list logo
