Re: [v8-dev] Event Loop context in the debugger

2018-11-28 Thread yangguo via v8-dev
I think the correct way for this is to rather implement a new devtools domain and implement support for the functionality in node itself. I think it would be backwards if that has to go through V8, since V8 itself has no concept of event loop aside from the microtasks queue. It would be

[v8-dev] Re: CreateForFunction() asserts for a C++ Function property value

2018-10-19 Thread yangguo via v8-dev
Fix coming up: https://chromium-review.googlesource.com/c/v8/v8/+/1291372 Yang On Tuesday, October 16, 2018 at 7:52:07 AM UTC+2, Dan Pike wrote: > > Does anyone use untemplated properties that store C++ Function callbacks? > Can you tell me what I’m doing wrong here, because I find that

[v8-dev] Re: [es6] parse destructuring assignment (issue 1309813007 by caitpotte...@gmail.com)

2015-11-24 Thread yangguo
On 2015/11/25 01:41:13, Yang wrote: On 2015/11/25 00:04:51, caitp wrote: > On 2015/11/24 21:37:29, caitp wrote: > > +yang, do you have an idea what would cause the DCHECK failure in > > VerifyRecompiledCode()? It doesn't seem to break the code when DCHECKS are > > disabled. > > More context,

[v8-dev] Re: [es6] parse destructuring assignment (issue 1309813007 by caitpotte...@gmail.com)

2015-11-24 Thread yangguo
On 2015/11/25 00:04:51, caitp wrote: On 2015/11/24 21:37:29, caitp wrote: > +yang, do you have an idea what would cause the DCHECK failure in > VerifyRecompiledCode()? It doesn't seem to break the code when DCHECKS are > disabled. More context, old_target->kind() == BUILTIN, while

[v8-dev] Re: Implement tracing interface for v8 (issue 988893003 by fmea...@chromium.org)

2015-11-19 Thread yangguo
LGTM with comments. I think I'm missing a test case where we can distinguish trace events by isolate (using the isolate as id?). https://codereview.chromium.org/988893003/diff/320001/include/v8-platform.h File include/v8-platform.h (right):

[v8-dev] Re: Revert of Ignore test failure for mjsunit/for-in-opt in gc stress. (issue 1419823009 by verwa...@chromium.org)

2015-10-26 Thread yangguo
On 2015/10/26 12:08:53, commit-bot: I haz the power wrote: Failed to apply patch for test/mjsunit/mjsunit.status: While running git apply --index -3 -p1; error: patch failed: test/mjsunit/mjsunit.status:322 Falling back to three-way merge... Applied patch to

[v8-dev] Re: Fix AstPrinter::VisitCallRuntime to not print garbage. (issue 1329133002 by mstarzin...@chromium.org)

2015-09-08 Thread yangguo
On 2015/09/08 11:11:21, Michael Starzinger wrote: Benedikt: PTAL Yang: FYI lgtm https://codereview.chromium.org/1329133002/ -- -- 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] Re: Version 4.6.85.14 (cherry-pick) (issue 1324153005 by yu...@chromium.org)

2015-09-08 Thread yangguo
On 2015/09/08 18:17:49, yurys wrote: Committed patchset #1 (id:1) manually as ff223e5a24342a6567c1e57ac58f8a2ec134f50f. lgtm. https://codereview.chromium.org/1324153005/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message

[v8-dev] Re: Cache String.split not found results as well (issue 1308373005 by k...@skomski.com)

2015-09-08 Thread yangguo
On 2015/09/08 09:50:39, Jakob wrote: LGTM. Yang, is more caching something we want strategically? I wouldn't be surprised if this cache were a benchmark-specific hack that we'd rather get rid of. If you don't like this change, feel free to revert it. Seems alright to me. We have this

[v8-dev] Re: Code::WipeOutHeader needs to null out the next code link to avoid (issue 1310503006 by o...@chromium.org)

2015-09-02 Thread yangguo
On 2015/09/02 12:33:53, rmcilroy wrote: lgtm with a question for Hannes. https://codereview.chromium.org/1310503006/diff/1/src/objects-inl.h File src/objects-inl.h (right): https://codereview.chromium.org/1310503006/diff/1/src/objects-inl.h#newcode6420 src/objects-inl.h:6420:

[v8-dev] Re: Make isolate.h usable without objects-inl.h header. (issue 1322883002 by mstarzin...@chromium.org)

2015-08-31 Thread yangguo
On 2015/08/31 19:10:57, Benedikt Meurer wrote: > Also, I am looking for a "regexp" OWNER that is _not_ on vacation right now. :( Hm, wait I think I mentioned that this "process thing" will cause exactly these kinds of problems, yeah I'm pretty sure I did. I guess there is only one word

[v8-dev] Re: [futex] Allow debugger to break in the middle of a futexWait (issue 1321543004 by bi...@chromium.org)

2015-08-31 Thread yangguo
On 2015/08/31 21:52:21, Michael Starzinger wrote: Some concerns. Also adding Jaro who might have an opinion about this. https://codereview.chromium.org/1321543004/diff/20001/src/execution.h File src/execution.h (right):

[v8-dev] Re: Postpone interrupts while dipatching debugger events to listeners (issue 1321263002 by yu...@chromium.org)

2015-08-31 Thread yangguo
On 2015/08/31 21:45:07, commit-bot: I haz the power wrote: Dry run: This issue passed the CQ dry run. lgtm https://codereview.chromium.org/1321263002/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are

[v8-dev] Re: [futex] Allow debugger to break in the middle of a futexWait (issue 1321543004 by bi...@chromium.org)

2015-08-31 Thread yangguo
yup. thats what I had in mind. https://codereview.chromium.org/1321543004/diff/40001/src/debug/debug.cc File src/debug/debug.cc (right): https://codereview.chromium.org/1321543004/diff/40001/src/debug/debug.cc#newcode2282 src/debug/debug.cc:2282: break_in_nondebuggable_)) please add brackets

[v8-dev] Re: Extract common debugger code for processing compile events (issue 1316213005 by yu...@chromium.org)

2015-08-31 Thread yangguo
lgtm https://codereview.chromium.org/1316213005/ -- -- 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

[v8-dev] Native context: install array methods via runtime import. (issue 1324483002 by yang...@chromium.org)

2015-08-28 Thread yangguo
Reviewers: cbruni, Message: Please review. Description: Native context: install array methods via runtime import. R=cbr...@chromium.org Please review this at https://codereview.chromium.org/1324483002/ Base URL: https://chromium.googlesource.com/v8/v8.git@master Affected files (+154, -188

[v8-dev] Re: [es6] Implement spec compliant ToName (actually ToPropertyKey). (issue 1319133002 by bmeu...@chromium.org)

2015-08-28 Thread yangguo
On 2015/08/28 09:23:12, commit-bot: I haz the power wrote: Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1319133002/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1319133002/1 lgtm.

[v8-dev] Re: [es6] Implement spec compliant ToPrimitive in the runtime. (issue 1306303003 by bmeu...@chromium.org)

2015-08-28 Thread yangguo
lgtm. https://codereview.chromium.org/1306303003/ -- -- 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

[v8-dev] Re: [runtime] Replace %to_string_fun with %_ToString. (issue 1323543002 by bmeu...@chromium.org)

2015-08-28 Thread yangguo
lgtm with a comment. https://codereview.chromium.org/1323543002/diff/1/test/mjsunit/compiler/jsnatives.js File test/mjsunit/compiler/jsnatives.js (left): https://codereview.chromium.org/1323543002/diff/1/test/mjsunit/compiler/jsnatives.js#oldcode32 test/mjsunit/compiler/jsnatives.js:32:

[v8-dev] Re: [runtime] Add %ToString and %_ToString and remove the TO_STRING builtin. (issue 1319973007 by bmeu...@chromium.org)

2015-08-28 Thread yangguo
On 2015/08/28 12:15:23, Benedikt Meurer wrote: Hey Yang, Next step to make builtins object obsolete. More to come in followup CLs. Please take a look. Thanks, Benedikt lgtm. https://codereview.chromium.org/1319973007/ -- -- v8-dev mailing list v8-dev@googlegroups.com

[v8-dev] Re: [V8] Report JSON parser script to DevTools (issue 1308123006 by kozyatins...@chromium.org)

2015-08-27 Thread yangguo
On 2015/08/27 18:22:06, kozyatinskiy wrote: All done. Please take a look. https://codereview.chromium.org/1308123006/diff/1/src/json-parser.h File src/json-parser.h (right): https://codereview.chromium.org/1308123006/diff/1/src/json-parser.h#newcode249 src/json-parser.h:249:

[v8-dev] [futex] Allow debugger to break in the middle of a futexWait (issue 1321543004 by bi...@chromium.org)

2015-08-27 Thread yangguo
https://codereview.chromium.org/1321543004/diff/1/src/debug/debug.cc File src/debug/debug.cc (right): https://codereview.chromium.org/1321543004/diff/1/src/debug/debug.cc#newcode2281 src/debug/debug.cc:2281: HandleJSFunction jsfun(JSFunction::cast(fun)); I think it might be cleaner to just pass

[v8-dev] Re: [V8] Report JSON parser script to DevTools (issue 1308123006 by kozyatins...@chromium.org)

2015-08-27 Thread yangguo
https://codereview.chromium.org/1308123006/diff/1/src/json-parser.h File src/json-parser.h (right): https://codereview.chromium.org/1308123006/diff/1/src/json-parser.h#newcode249 src/json-parser.h:249: isolate()-debug()-OnAfterCompile(script); And please add a comment on why this is necessary.

[v8-dev] Re: Implement tracing interface for v8 (issue 988893003 by fmea...@chromium.org)

2015-08-27 Thread yangguo
On 2015/08/25 22:02:46, caseq wrote: https://codereview.chromium.org/988893003/diff/260001/include/v8-tracing.h File include/v8-tracing.h (right): https://codereview.chromium.org/988893003/diff/260001/include/v8-tracing.h#newcode49 include/v8-tracing.h:49: virtual EventTracer::Handle

[v8-dev] Re: Adding ElementsAccessor Splice (issue 1312033003 by cbr...@chromium.org)

2015-08-27 Thread yangguo
lgtm with comments. https://codereview.chromium.org/1312033003/diff/60001/src/builtins.cc File src/builtins.cc (right): https://codereview.chromium.org/1312033003/diff/60001/src/builtins.cc#newcode169 src/builtins.cc:169: // This is an extended version of ECMA-262 9.4, but additionally This

[v8-dev] Re: [heap] Limit friendship of the Heap class to essentials. (issue 1320843002 by mstarzin...@chromium.org)

2015-08-27 Thread yangguo
On 2015/08/27 09:27:48, Michael Starzinger wrote: https://codereview.chromium.org/1320843002/diff/1/src/heap/heap.h File src/heap/heap.h (right): https://codereview.chromium.org/1320843002/diff/1/src/heap/heap.h#newcode1231 src/heap/heap.h:1231: void public_set_code_stub_context(Object*

[v8-dev] Re: [es6] Initial steps towards a correct implementation of IsCallable. (issue 1316933002 by bmeu...@chromium.org)

2015-08-27 Thread yangguo
LGTM. I haven't checked for spec-compliance. Please also run layout tests. https://codereview.chromium.org/1316933002/diff/80001/src/hydrogen-instructions.cc File src/hydrogen-instructions.cc (right):

[v8-dev] Re: Move (uppercase) JS builtins from js builtins object to native context. (issue 1316943002 by yang...@chromium.org)

2015-08-27 Thread yangguo
Added platform ports and unittest for interpreter assembler. Do you guys want to look at this again? https://codereview.chromium.org/1316943002/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to

[v8-dev] Re: Vector ICs: Ensure KeyedAccessStore mode is encoded in all handlers. (issue 1312693004 by mvstan...@chromium.org)

2015-08-26 Thread yangguo
On 2015/08/26 09:33:46, mvstanton wrote: Hi Yang, Here is the CL we discussed. Validates/protects that the handlers have the same store mode as the IC extra state bits. lgtm. https://codereview.chromium.org/1312693004/ -- -- v8-dev mailing list v8-dev@googlegroups.com

[v8-dev] Re: [heap] More flag cleanup. (issue 1314863003 by mlippa...@chromium.org)

2015-08-26 Thread yangguo
On 2015/08/25 17:13:49, Michael Lippautz wrote: +yang for debug/* https://codereview.chromium.org/1314863003/diff/40001/src/heap/heap.cc File src/heap/heap.cc (right): https://codereview.chromium.org/1314863003/diff/40001/src/heap/heap.cc#newcode825 src/heap/heap.cc:825:

[v8-dev] Re: [runtime] Remove the redundant %_IsObject intrinsic. (issue 1313903003 by bmeu...@chromium.org)

2015-08-26 Thread yangguo
On 2015/08/26 10:31:19, Benedikt Meurer wrote: Hey Michi, Removing redundant code... Please take a look. Thanks, Benedikt There goes another 1k LoC :D LGTM for runtime/* and full-codegen/* https://codereview.chromium.org/1313903003/ -- -- v8-dev mailing list v8-dev@googlegroups.com

[v8-dev] Re: Call JS functions via native context instead of js builtins object. (issue 1306993003 by yang...@chromium.org)

2015-08-26 Thread yangguo
On 2015/08/25 18:30:01, Yang wrote: On 2015/08/25 18:12:09, Michael Starzinger wrote: Essentially one high-level comment/suggestion. Happy to discuss in person and/or be convinced otherwise. Would also be interested in what Benedikt thinks about this.

[v8-dev] Re: [heap] User safer root set accessor when possible. (issue 1312763006 by mstarzin...@chromium.org)

2015-08-26 Thread yangguo
lgtm https://codereview.chromium.org/1312763006/diff/1/src/snapshot/serialize.cc File src/snapshot/serialize.cc (right): https://codereview.chromium.org/1312763006/diff/1/src/snapshot/serialize.cc#newcode956 src/snapshot/serialize.cc:956: int id = source_.GetInt();

[v8-dev] Re: Call JS functions via native context instead of js builtins object. (issue 1306993003 by yang...@chromium.org)

2015-08-26 Thread yangguo
Addressed comments. https://codereview.chromium.org/1306993003/diff/20001/src/bootstrapper.cc File src/bootstrapper.cc (right): https://codereview.chromium.org/1306993003/diff/20001/src/bootstrapper.cc#newcode1847 src/bootstrapper.cc:1847: native_context()-set_reflect_construct(*construct); On

[v8-dev] Install js intrinsic fallbacks for array functions on the native context. (issue 1309503003 by yang...@chromium.org)

2015-08-26 Thread yangguo
Reviewers: cbruni, Description: Install js intrinsic fallbacks for array functions on the native context. R=cbr...@chromium.org Please review this at https://codereview.chromium.org/1309503003/ Base URL: https://chromium.googlesource.com/v8/v8.git@context_intrinsics Affected files (+55, -58

[v8-dev] Re: Move (uppercase) JS builtins from js builtins object to native context. (issue 1316943002 by yang...@chromium.org)

2015-08-26 Thread yangguo
On 2015/08/26 14:16:42, Michael Starzinger wrote: LGTM modulo architecture ports. Didn't look at interpreter though. https://codereview.chromium.org/1316943002/diff/1/src/compiler/ast-graph-builder.cc File src/compiler/ast-graph-builder.cc (right):

[v8-dev] Re: Remove named load from builtin in default super call. (issue 1314493006 by mstarzin...@chromium.org)

2015-08-26 Thread yangguo
On 2015/08/26 14:21:24, Michael Starzinger wrote: Addressed offline comment. PTAL. LGTM. Please update comment above your change. The function is not called $reflectConstruct anymore. https://codereview.chromium.org/1314493006/ -- -- v8-dev mailing list v8-dev@googlegroups.com

[v8-dev] Move (uppercase) JS builtins from js builtins object to native context. (issue 1316943002 by yang...@chromium.org)

2015-08-26 Thread yangguo
Reviewers: Benedikt Meurer, Michael Starzinger, rmcilroy, Message: Can I get a round of comments before porting? Description: Move (uppercase) JS builtins from js builtins object to native context. R=bmeu...@chromium.org, mstarzin...@chromium.org, rmcil...@chromium.org Please review this at

[v8-dev] Call JS functions via native context instead of js builtins object. (issue 1306993003 by yang...@chromium.org)

2015-08-25 Thread yangguo
Reviewers: Benedikt Meurer, Michael Starzinger, Message: I'd like some comments on the ia32 port. Description: Call JS functions via native context instead of js builtins object. For this, we introduce a new runtime function intrinsic type, which represent native context slots.

[v8-dev] Re: Array.prototype.unshift builtin improvements (issue 1311343002 by cbr...@chromium.org)

2015-08-25 Thread yangguo
On 2015/08/25 08:29:53, cbruni wrote: PTAL lgtm. https://codereview.chromium.org/1311343002/ -- -- 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

[v8-dev] Re: Call JS functions via native context instead of js builtins object. (issue 1306993003 by yang...@chromium.org)

2015-08-25 Thread yangguo
On 2015/08/25 18:12:09, Michael Starzinger wrote: Essentially one high-level comment/suggestion. Happy to discuss in person and/or be convinced otherwise. Would also be interested in what Benedikt thinks about this. https://codereview.chromium.org/1306993003/diff/1/src/ast.h File

[v8-dev] Re: Add a PLACEHOLDER code kind. (issue 1308393003 by tit...@chromium.org)

2015-08-25 Thread yangguo
On 2015/08/24 19:51:52, titzer wrote: LGTM. I wonder whether we can know the code kind upfront, even if the code object does not exist yet. And where do we need the code kind when calling into a code object that does not yet exist? https://codereview.chromium.org/1308393003/ -- -- v8-dev

[v8-dev] Re: Make Simulator respect C stack limits as well. (issue 1314623002 by mstarzin...@chromium.org)

2015-08-24 Thread yangguo
lgtm. https://codereview.chromium.org/1314623002/ -- -- 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

[v8-dev] Re: Move StackGuard::InterruptRequested into StackLimitCheck. (issue 1310253002 by mstarzin...@chromium.org)

2015-08-24 Thread yangguo
lgtm https://codereview.chromium.org/1310253002/diff/1/src/json-parser.h File src/json-parser.h (right): https://codereview.chromium.org/1310253002/diff/1/src/json-parser.h#newcode267 src/json-parser.h:267: ExecutionAccess access(isolate_); On 2015/08/24 14:40:06, Michael Starzinger wrote:

[v8-dev] Re: Native context: alpha sort slots and remove boilerplate. (issue 1307963002 by yang...@chromium.org)

2015-08-24 Thread yangguo
On 2015/08/21 15:51:32, Michael Starzinger wrote: LGTM, just a naming suggestion. https://codereview.chromium.org/1307963002/diff/1/src/contexts.h File src/contexts.h (right): https://codereview.chromium.org/1307963002/diff/1/src/contexts.h#newcode76 src/contexts.h:76: #define

[v8-dev] Re: Parse arrow functions at proper precedence level (issue 1286383005 by wi...@igalia.com)

2015-08-24 Thread yangguo
A revert of this CL (patchset #2 id:60001) has been created in https://codereview.chromium.org/1315503002/ by yang...@chromium.org. The reason for reverting is: Breaks layout test. Please change test expectation on blink first. ---

[v8-dev] Revert of Parse arrow functions at proper precedence level (issue 1315503002 by yang...@chromium.org)

2015-08-24 Thread yangguo
Reviewers: rossberg, Michael Starzinger, fennyfanny655, Michael Achenbach (OOO), wingo, Message: Created Revert of Parse arrow functions at proper precedence level Description: Revert of Parse arrow functions at proper precedence level (patchset #2 id:60001 of

[v8-dev] Re: [Interpreter] Allow verification and trace-turbo for bytecode handlers. (issue 1297203002 by rmcil...@chromium.org)

2015-08-24 Thread yangguo
On 2015/08/22 00:08:44, commit-bot: I haz the power wrote: Dry run: This issue passed the CQ dry run. serializer change lgtm. https://codereview.chromium.org/1297203002/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message

[v8-dev] Re: Get rid of CompilationInfo::GenerateCodeStub method. (issue 1309883002 by mstarzin...@chromium.org)

2015-08-24 Thread yangguo
lgtm lgtm https://codereview.chromium.org/1309883002/ -- -- 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

[v8-dev] Re: Correctify instanceof and make it optimizable. (issue 1304633002 by bmeu...@chromium.org)

2015-08-24 Thread yangguo
On 2015/08/21 10:32:35, Michael Starzinger wrote: LGTM on TurboFan, CodeFactory and InterfaceDescriptor. Didn't look at the rest. LGTM with a comment. Please wait for the MIPS port before landing. https://codereview.chromium.org/1304633002/ -- -- v8-dev mailing list v8-dev@googlegroups.com

[v8-dev] Re: Correctify instanceof and make it optimizable. (issue 1304633002 by bmeu...@chromium.org)

2015-08-24 Thread yangguo
... and the comment https://codereview.chromium.org/1304633002/diff/180001/src/arm/code-stubs-arm.cc File src/arm/code-stubs-arm.cc (right): https://codereview.chromium.org/1304633002/diff/180001/src/arm/code-stubs-arm.cc#newcode1305 src/arm/code-stubs-arm.cc:1305: // Lookup the {function} and

[v8-dev] Re: [heap] Move RegExpResultCache out of the heap. (issue 1306053003 by mstarzin...@chromium.org)

2015-08-21 Thread yangguo
On 2015/08/21 12:10:42, Michael Starzinger wrote: lgtm. https://codereview.chromium.org/1306053003/ -- -- 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

[v8-dev] Re: Small MessageLocation related refactoring. (issue 1309673003 by yang...@chromium.org)

2015-08-21 Thread yangguo
On 2015/08/21 12:12:21, cbruni wrote: lgtm https://codereview.chromium.org/1309673003/diff/1/src/messages.cc File src/messages.cc (right): https://codereview.chromium.org/1309673003/diff/1/src/messages.cc#newcode49 src/messages.cc:49: script_handle =

[v8-dev] Re: Do not use js builtins object to determine whether a function is a builtin. (issue 1292283004 by yang...@chromium.org)

2015-08-21 Thread yangguo
Michael, please take a look. Thanks. https://codereview.chromium.org/1292283004/ -- -- 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

[v8-dev] Re: [es6] Parameter scopes for sloppy eval (issue 1292753007 by rossb...@chromium.org)

2015-08-20 Thread yangguo
On 2015/08/19 16:10:23, rossberg wrote: Very confused. In the example, is the 'x' in the default parameter arrow function supposed to bind to the 'x' introduced later in eval? Wtf. https://codereview.chromium.org/1292753007/ -- -- v8-dev mailing list v8-dev@googlegroups.com

[v8-dev] Re: [es6] Parameter scopes for sloppy eval (issue 1292753007 by rossb...@chromium.org)

2015-08-20 Thread yangguo
On 2015/08/20 09:09:22, Yang wrote: On 2015/08/20 09:08:56, Yang wrote: On 2015/08/19 16:10:23, rossberg wrote: Very confused. In the example, is the 'x' in the default parameter arrow function supposed to bind to the 'x' introduced later in eval? Wtf. Wait nvm. debugger part lgtm.

[v8-dev] Re: [es6] Parameter scopes for sloppy eval (issue 1292753007 by rossb...@chromium.org)

2015-08-20 Thread yangguo
On 2015/08/20 09:08:56, Yang wrote: On 2015/08/19 16:10:23, rossberg wrote: Very confused. In the example, is the 'x' in the default parameter arrow function supposed to bind to the 'x' introduced later in eval? Wtf. Wait nvm. https://codereview.chromium.org/1292753007/ -- -- v8-dev

[v8-dev] Re: Remove grab-bag includes of v8.h from everywhere. (issue 1285183010 by mstarzin...@chromium.org)

2015-08-20 Thread yangguo
On 2015/08/20 05:35:07, Benedikt Meurer wrote: lgtm lgtm. https://codereview.chromium.org/1285183010/ -- -- 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

[v8-dev] Re: Re-enable regress-crbug-501711 and regress-4279 for --isolates tests (issue 1305583002 by bi...@chromium.org)

2015-08-20 Thread yangguo
On 2015/08/19 21:00:00, binji wrote: Committed patchset #1 (id:1) to pending queue manually as 38181c387b7f0e2e9ed0b09f83304b2e871e23ab (presubmit successful). lgtm. https://codereview.chromium.org/1305583002/ -- -- v8-dev mailing list v8-dev@googlegroups.com

[v8-dev] Re: Separate UnicodeCache out into an own file. (issue 1287893006 by mstarzin...@chromium.org)

2015-08-20 Thread yangguo
On 2015/08/20 11:11:36, Michael Starzinger wrote: lgtm. https://codereview.chromium.org/1287893006/ -- -- 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

[v8-dev] Re: Unify symbols sharing across native scripts and runtime. (issue 1293493004 by yang...@chromium.org)

2015-08-20 Thread yangguo
On 2015/08/19 13:51:49, Yang wrote: On 2015/08/19 13:41:13, Michael Lippautz wrote: heap lgtm https://codereview.chromium.org/1293493004/diff/1/src/heap/heap.h File src/heap/heap.h (right): https://codereview.chromium.org/1293493004/diff/1/src/heap/heap.h#newcode297

[v8-dev] Re: Introduce SharedFunctionInfo::Iterator and Script::Iterator. (issue 1300333003 by yang...@chromium.org)

2015-08-20 Thread yangguo
Added an iterator for WeakFixedArray, so that we can use it whenever we want to iterated over one, hiding some complexity. https://codereview.chromium.org/1300333003/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because

[v8-dev] Re: Make snapshot.h usable without objects-inl.h header. (issue 1287113010 by mstarzin...@chromium.org)

2015-08-20 Thread yangguo
On 2015/08/20 12:36:35, Michael Starzinger wrote: lgtm. https://codereview.chromium.org/1287113010/ -- -- 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

[v8-dev] Re: Unify symbols sharing across native scripts and runtime. (issue 1293493004 by yang...@chromium.org)

2015-08-20 Thread yangguo
On 2015/08/20 12:50:12, rossberg wrote: LGTM It's sad that this makes the use of private symbols unmodular. But I guess uniformity wins... The upside is that we would share private symbols across different native contexts instead of allocating new ones. This saves us from ICs going

[v8-dev] Re: Simplify KeyedAccessStoreMode. (issue 1303813004 by mvstan...@chromium.org)

2015-08-20 Thread yangguo
On 2015/08/20 13:44:53, mvstanton wrote: Hi Yang, Here is a cleanup discovered on my vector store path... lgtm. https://codereview.chromium.org/1303813004/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are

[v8-dev] Re: Unify symbols sharing across native scripts and runtime. (issue 1293493004 by yang...@chromium.org)

2015-08-20 Thread yangguo
On 2015/08/20 12:50:12, rossberg wrote: LGTM It's sad that this makes the use of private symbols unmodular. But I guess uniformity wins... https://codereview.chromium.org/1293493004/diff/1/src/array-iterator.js File src/array-iterator.js (right):

[v8-dev] Re: Unify symbols sharing across native scripts and runtime. (issue 1293493004 by yang...@chromium.org)

2015-08-19 Thread yangguo
On 2015/08/19 13:41:13, Michael Lippautz wrote: heap lgtm https://codereview.chromium.org/1293493004/diff/1/src/heap/heap.h File src/heap/heap.h (right): https://codereview.chromium.org/1293493004/diff/1/src/heap/heap.h#newcode297 src/heap/heap.h:297: #define PRIVATE_SYMBOL_LIST(V)

[v8-dev] Re: Cleanup: Remove unncessary leave_frame parameter from stub cache. (issue 1299213002 by mvstan...@chromium.org)

2015-08-19 Thread yangguo
On 2015/08/19 13:05:35, mvstanton wrote: Hi Yang, This parameter is a left-over from the days when vector-based loads were flagged. Now it gets in the way. Time to clean up! --Michael lgtm https://codereview.chromium.org/1299213002/ -- -- v8-dev mailing list v8-dev@googlegroups.com

[v8-dev] Re: Do not compact weak fixed array when re-allocating new backing store. (issue 1294883004 by yang...@chromium.org)

2015-08-19 Thread yangguo
On 2015/08/19 13:52:12, mvstanton wrote: LGTM with the one nit. https://codereview.chromium.org/1294883004/diff/1/src/objects.cc File src/objects.cc (right): https://codereview.chromium.org/1294883004/diff/1/src/objects.cc#newcode7868 src/objects.cc:7868: } How about calling

[v8-dev] Re: Rename ParserInfo::function() and CompilationInfo::function() to literal(). (issue 1301583005 by tit...@chromium.org)

2015-08-19 Thread yangguo
On 2015/08/19 16:47:21, Michael Starzinger wrote: LGTM on full-codegen. lgtm on debug https://codereview.chromium.org/1301583005/ -- -- 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

[v8-dev] Re: Ignore test failure for mjsunit/for-in-opt in gc stress. (issue 1295513004 by yang...@chromium.org)

2015-08-19 Thread yangguo
On 2015/08/19 09:31:44, Hablich wrote: On 2015/08/19 09:31:13, Hablich wrote: On 2015/08/19 09:30:14, commit-bot: I haz the power wrote: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1295513004/1 View timeline at

[v8-dev] Re: Native context: install JS builtins via container object. (issue 1296163003 by yang...@chromium.org)

2015-08-19 Thread yangguo
On 2015/08/19 08:52:44, commit-bot: I haz the power wrote: Dry run: This issue passed the CQ dry run. ping :) https://codereview.chromium.org/1296163003/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are

[v8-dev] Re: Put V8 extras into the snapshot (issue 1289603002 by dome...@chromium.org)

2015-08-18 Thread yangguo
On 2015/08/17 17:51:27, domenic wrote: On 2015/08/17 at 16:47:26, mstarzinger wrote: https://codereview.chromium.org/1289603002/diff/170001/src/snapshot/natives.h File src/snapshot/natives.h (right): https://codereview.chromium.org/1289603002/diff/170001/src/snapshot/natives.h#newcode9

[v8-dev] Re: Remove property loads from js builtins objects from runtime. (issue 1293113002 by yang...@chromium.org)

2015-08-18 Thread yangguo
A revert of this CL (patchset #2 id:20001) has been created in https://codereview.chromium.org/1297803003/ by yang...@chromium.org. The reason for reverting is: Still failures in debug-isolates tests. https://codereview.chromium.org/1293113002/ -- -- v8-dev mailing list v8-dev@googlegroups.com

[v8-dev] Re: Remove inline header includes from natives.h header. (issue 1303463002 by mstarzin...@chromium.org)

2015-08-18 Thread yangguo
On 2015/08/18 12:05:29, Michael Starzinger wrote: lgtm if buildbots agree! https://codereview.chromium.org/1303463002/ -- -- 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

[v8-dev] Revert of Remove property loads from js builtins objects from runtime. (issue 1297803003 by yang...@chromium.org)

2015-08-18 Thread yangguo
Reviewers: cbruni, Hannes Payer, Message: Created Revert of Remove property loads from js builtins objects from runtime. Description: Revert of Remove property loads from js builtins objects from runtime. (patchset #2 id:20001 of https://codereview.chromium.org/1293113002/ ) Reason for

[v8-dev] Reland of move property loads from js builtins objects from runtime. (issue 1298733003 by yang...@chromium.org)

2015-08-18 Thread yangguo
Reviewers: cbruni, Hannes Payer, Message: Created Reland of move property loads from js builtins objects from runtime. Description: Reland of move property loads from js builtins objects from runtime. (patchset #1 id:1 of https://codereview.chromium.org/1297803003/ ) Reason for revert:

[v8-dev] Re: Revert of Remove property loads from js builtins objects from runtime. (issue 1297803003 by yang...@chromium.org)

2015-08-18 Thread yangguo
A revert of this CL (patchset #1 id:1) has been created in https://codereview.chromium.org/1298733003/ by yang...@chromium.org. The reason for reverting is: Debug isolate failure has nothing to do with this CL.. https://codereview.chromium.org/1297803003/ -- -- v8-dev mailing list

[v8-dev] Re: Remove grab-bag includes of v8.h from architecture ports. (issue 1299563003 by mstarzin...@chromium.org)

2015-08-17 Thread yangguo
On 2015/08/17 09:17:13, Michael Starzinger wrote: lgtm https://codereview.chromium.org/1299563003/ -- -- 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

[v8-dev] Re: Do not use js builtins object when constructing an error. (issue 1295093002 by yang...@chromium.org)

2015-08-17 Thread yangguo
This follows the pattern of putting the function into a native context slot instead of retrieving it from the js builtins object. Also removes some dead code. https://codereview.chromium.org/1295093002/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev ---

[v8-dev] Re: Add experimental, non-snapshotted V8 extras (issue 1284413002 by dome...@chromium.org)

2015-08-17 Thread yangguo
Hannes or Michael, could one of you review the heap changes? https://codereview.chromium.org/1284413002/ -- -- 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

[v8-dev] Re: [runtime] Unify and fix the strict equality comparison. (issue 1298603002 by bmeu...@chromium.org)

2015-08-17 Thread yangguo
LGTM. https://codereview.chromium.org/1298603002/diff/1/src/api.cc File src/api.cc (left): https://codereview.chromium.org/1298603002/diff/1/src/api.cc#oldcode3431 src/api.cc:3431: return other-IsUndefined() || other-IsUndetectableObject(); This undetectable check is lost. I guess this is

[v8-dev] Re: Debugger: simplify calling into Javascript. (issue 1292533003 by yang...@chromium.org)

2015-08-14 Thread yangguo
On 2015/08/13 14:57:36, Jakob wrote: LGTM https://codereview.chromium.org/1292533003/diff/1/src/debug/debug.cc File src/debug/debug.cc (right): https://codereview.chromium.org/1292533003/diff/1/src/debug/debug.cc#newcode1959 src/debug/debug.cc:1959: HandleObject argv[] =

[v8-dev] Re: [runtime] Remove useless IN builtin. (issue 1295433002 by bmeu...@chromium.org)

2015-08-14 Thread yangguo
On 2015/08/14 05:06:34, Benedikt Meurer wrote: On 2015/08/13 18:04:50, adamk wrote: The CL description says this builtin is useless, but the fast path for arrays was added as a fix for a performance bug: https://code.google.com/p/v8/issues/detail?id=3701 If this removal is part of a broader

[v8-dev] Re: Put V8 extras into the snapshot (issue 1289603002 by dome...@chromium.org)

2015-08-14 Thread yangguo
lgtm with small nit https://codereview.chromium.org/1289603002/diff/140001/src/snapshot/serialize.h File src/snapshot/serialize.h (right): https://codereview.chromium.org/1289603002/diff/140001/src/snapshot/serialize.h#newcode404 src/snapshot/serialize.h:404: static const int kVariableRepeat =

[v8-dev] Re: Add experimental, non-snapshotted V8 extras (issue 1284413002 by dome...@chromium.org)

2015-08-14 Thread yangguo
On 2015/08/13 18:35:43, domenic wrote: On 2015/08/13 at 12:35:31, yangguo wrote: LGTM, but it would be very nice if we had a test case for experimental extras as well. Test added. Looks like it won't let me run tryjobs until the previous patch lands though. still lgtm. https

[v8-dev] Re: Remove grab-bag includes of v8.h from regexp engine. (issue 1294783002 by mstarzin...@chromium.org)

2015-08-14 Thread yangguo
On 2015/08/14 13:59:24, Michael Starzinger wrote: lgtm. https://codereview.chromium.org/1294783002/ -- -- 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

[v8-dev] Re: Remove grab-bag includes of v8.h from several files. (issue 1297583002 by mstarzin...@chromium.org)

2015-08-14 Thread yangguo
On 2015/08/14 08:51:21, Michael Starzinger wrote: lgtm. https://codereview.chromium.org/1297583002/ -- -- 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

[v8-dev] Re: Add experimental, non-snapshotted V8 extras (issue 1284413002 by dome...@chromium.org)

2015-08-14 Thread yangguo
On 2015/08/14 19:33:57, domenic wrote: On 2015/08/14 at 19:28:17, commit-bot wrote: Try jobs failed on following builders: v8_linux64_avx2_rel on tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_avx2_rel/builds/3287) v8_linux_rel on tryserver.v8

[v8-dev] Re: [serializer] Move WeakFixedArray compaction to separate heap walk phase (issue 1290393002 by jkumme...@chromium.org)

2015-08-14 Thread yangguo
On 2015/08/14 12:41:33, Jakob wrote: As discussed. For the Scripts' weak fixed array, it shouldn't matter when we clear them; I moved clearing those as well for consistency. lgtm. https://codereview.chromium.org/1290393002/ -- -- v8-dev mailing list v8-dev@googlegroups.com

[v8-dev] Debugger: use a Map to cache mirrors. (issue 1287243002 by yang...@chromium.org)

2015-08-13 Thread yangguo
Reviewers: Benedikt Meurer, Description: Debugger: use a Map to cache mirrors. This makes mirror cache lookup O(1) instead of O(n). The downside is that the lookup via handle is O(n). This is fine because handles are only used in the JSON api, which is not used by Chrome and on death row.

[v8-dev] Debugger: do not expose global object. (issue 1290063002 by yang...@chromium.org)

2015-08-13 Thread yangguo
Reviewers: Benedikt Meurer, Message: This is necessary for https://codereview.chromium.org/1287243002, because we can't use global objects as map keys. Description: Debugger: do not expose global object. Please review this at https://codereview.chromium.org/1290063002/ Base URL:

[v8-dev] Re: Add more OWNERS for components. (issue 1293453002 by bmeu...@chromium.org)

2015-08-13 Thread yangguo
On 2015/08/13 07:34:48, Benedikt Meurer wrote: Hey Yang, As discussed offline. Please take a look. Thanks, Benedikt lgtm. https://codereview.chromium.org/1293453002/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message

[v8-dev] Re: Group lexical context variables for faster look up. (issue 1281883002 by yang...@chromium.org)

2015-08-13 Thread yangguo
A revert of this CL (patchset #2 id:20001) has been created in https://codereview.chromium.org/1290053002/ by yang...@chromium.org. The reason for reverting is: This performance hack is no longer necessary.. https://codereview.chromium.org/1281883002/ -- -- v8-dev mailing list

[v8-dev] Revert of Group lexical context variables for faster look up. (issue 1290053002 by yang...@chromium.org)

2015-08-13 Thread yangguo
Reviewers: rossberg, Message: Created Revert of Group lexical context variables for faster look up. Description: Revert of Group lexical context variables for faster look up. (patchset #2 id:20001 of https://codereview.chromium.org/1281883002/ ) Reason for revert: This performance hack is no

[v8-dev] Re: [runtime] Remove useless DELETE builtin. (issue 1291973002 by bmeu...@chromium.org)

2015-08-13 Thread yangguo
On 2015/08/13 08:52:58, Benedikt Meurer wrote: Hey Yang, Moar cleanup to the builtins. Please take a look. Thanks, Benedikt lgtm. https://codereview.chromium.org/1291973002/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this

[v8-dev] Debugger: remove duplicate heap iterations. (issue 1291043002 by yang...@chromium.org)

2015-08-13 Thread yangguo
Reviewers: Michael Lippautz, Message: Please take a look. This simplifies some debugger code without changing the functionality. Description: Debugger: remove duplicate heap iterations. R=mlippa...@chromium.org Please review this at https://codereview.chromium.org/1291043002/ Base URL:

[v8-dev] Re: Debugger: use a Map to cache mirrors. (issue 1287243002 by yang...@chromium.org)

2015-08-13 Thread yangguo
FYI yurys@ https://codereview.chromium.org/1287243002/ -- -- 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

[v8-dev] Re: Add experimental, non-snapshotted V8 extras (issue 1284413002 by dome...@chromium.org)

2015-08-13 Thread yangguo
LGTM, but it would be very nice if we had a test case for experimental extras as well. https://codereview.chromium.org/1284413002/ -- -- 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

[v8-dev] Re: [runtime] Remove useless IN builtin. (issue 1295433002 by bmeu...@chromium.org)

2015-08-13 Thread yangguo
LGTM if removed IN from builtins.h https://codereview.chromium.org/1295433002/diff/1/src/runtime/runtime-object.cc File src/runtime/runtime-object.cc (right): https://codereview.chromium.org/1295433002/diff/1/src/runtime/runtime-object.cc#newcode811 src/runtime/runtime-object.cc:811: Maybebool

  1   2   3   4   5   6   7   8   9   10   >