[v8-dev] Re: Generated code for substring slices in x64 and arm. (issue 7795018)

2011-08-31 Thread antonm
LGTM, but Erik and William might be better reviewers for ARM and x64. http://codereview.chromium.org/7795018/diff/1/src/arm/code-stubs-arm.cc File src/arm/code-stubs-arm.cc (right): http://codereview.chromium.org/7795018/diff/1/src/arm/code-stubs-arm.cc#newcode5658 src/arm/code-stubs-arm.cc:565

[v8-dev] Re: Generated code for substring slices in ia32. (issue 7744052)

2011-08-29 Thread antonm
32.cc:5728: const uint32_t kSlicedNotConsMask = kSlicedStringTag & ~kConsStringTag; On 2011/08/29 15:17:07, Yang wrote: On 2011/08/29 14:51:27, antonm wrote: > On 2011/08/26 16:10:55, antonm wrote: > > lift those constants into objects.h? > > any response? I did move these

[v8-dev] Re: Generated code for substring slices in ia32. (issue 7744052)

2011-08-29 Thread antonm
:55:56, Yang wrote: On 2011/08/26 16:10:55, antonm wrote: > do you want to fallback here from make_two_character_string? Yes. Brevity is :) I meant you know for sure you won't need a slice for two char string, so you're doing redundant work. http://codereview.chromium.org/7

[v8-dev] Re: Generated code for substring slices in ia32. (issue 7744052)

2011-08-26 Thread antonm
LGTM http://codereview.chromium.org/7744052/diff/1/src/ia32/code-stubs-ia32.cc File src/ia32/code-stubs-ia32.cc (right): http://codereview.chromium.org/7744052/diff/1/src/ia32/code-stubs-ia32.cc#newcode5707 src/ia32/code-stubs-ia32.cc:5707: __ SmiTag(edx); // Make edx a smi again. do you want

[v8-dev] Re: Tentative implementation of string slices (hidden under the flag --string-slices). (issue 7477045)

2011-08-26 Thread antonm
: On 2011/08/25 16:02:45, antonm wrote: > maybe use arg1? I don't quite follow. arg1 is not defined until way later. Never mind---my fault. Sorry. http://codereview.chromium.org/7477045/ -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev

[v8-dev] Re: Tentative implementation of string slices (hidden under the flag --string-slices). (issue 7477045)

2011-08-25 Thread antonm
http://codereview.chromium.org/7477045/diff/59001/src/heap.cc File src/heap.cc (right): http://codereview.chromium.org/7477045/diff/59001/src/heap.cc#newcode2651 src/heap.cc:2651: !ConsString::cast(buffer)->first()->IsSeqString())) || nit: please, indent this line for proper grouping http://cod

[v8-dev] Re: Tentative implementation of string slices (hidden under the flag --string-slices). (issue 7477045)

2011-08-25 Thread antonm
http://codereview.chromium.org/7477045/diff/52001/src/arm/code-stubs-arm.cc File src/arm/code-stubs-arm.cc (right): http://codereview.chromium.org/7477045/diff/52001/src/arm/code-stubs-arm.cc#newcode4343 src/arm/code-stubs-arm.cc:4343: __ mov(r9, Operand(0)); does it belong here? maybe move it

[v8-dev] Allows not API functions as inputs for CreationConext method. (issue7552034)

2011-08-10 Thread antonm
Reviewers: danno, Description: Allows not API functions as inputs for CreationConext method. Please review this at http://codereview.chromium.org/7552034/ SVN Base: https://v8.googlecode.com/svn/branches/bleeding_edge Affected files: M src/api.cc M test/cctest/test-api.cc Index: src/api

[v8-dev] Re: Tentative implementation of string slices (hidden under the flag --string-slices). (issue7477045)

2011-08-04 Thread antonm
LGTM Might be a good idea to add verification for sliced strings. http://codereview.chromium.org/7477045/diff/20075/src/arm/code-stubs-arm.cc File src/arm/code-stubs-arm.cc (right): http://codereview.chromium.org/7477045/diff/20075/src/arm/code-stubs-arm.cc#newcode5431 src/arm/code-stubs-arm.c

[v8-dev] Make Nan, Infinity and undefined on global object not overwritable. (issue7551013)

2011-08-02 Thread antonm
Reviewers: danno, Message: This make v8 behave like Safari does. And even as ECMAScript-262 mandates :) Surprisingly, I found no Sputnik tests which verify this behviour, but I only inspected the tests for the given chapters and didn't run the follow suite. Description: Make Nan, Infinit

[v8-dev] Re: Tentative implementation of string slices (hidden under the flag --string-slices). (issue7477045)

2011-07-27 Thread antonm
http://codereview.chromium.org/7477045/diff/6002/src/arm/code-stubs-arm.cc File src/arm/code-stubs-arm.cc (right): http://codereview.chromium.org/7477045/diff/6002/src/arm/code-stubs-arm.cc#newcode4804 src/arm/code-stubs-arm.cc:4804: __ tst(result_, Operand(kStringRepresentationMask)); Do we hav

[v8-dev] Re: Tentative implementation of string slices (hidden under the flag --string-slices). (issue7477045)

2011-07-27 Thread antonm
+VItalyr who did a lot of string work. First round of comments. http://codereview.chromium.org/7477045/diff/1/src/heap.cc File src/heap.cc (right): http://codereview.chromium.org/7477045/diff/1/src/heap.cc#newcode2687 src/heap.cc:2687: WriteBarrierMode mode = sliced_string->GetWriteBarrierMode

[v8-dev] Re: Refactor storage of global handles. (issue7054072)

2011-06-06 Thread antonm
DBC http://codereview.chromium.org/7054072/diff/1/src/global-handles.h File src/global-handles.h (right): http://codereview.chromium.org/7054072/diff/1/src/global-handles.h#newcode165 src/global-handles.h:165: // Iterates over all strong and dependent handles. nit: comment requires an update h

[v8-dev] Re: Handle changes to the Object prototype in fast handling of arrays (issue7067019)

2011-05-24 Thread antonm
ean JSObject::cast and != global_context->initial_object_prototype() check. But if don't allow anything except for null and JSObject as a prototype value, then it's not mandatory. On 2011/05/24 12:25:52, Søren Gjesse wrote: On 2011/05/24 10:38:20, antonm wrote: > while you're here

[v8-dev] Re: Move young independent handles to the end of the global handle's list. (issue7062004)

2011-05-24 Thread antonm
concerned with is_in_independent_tail_ On 2011/05/23 20:08:15, Vyacheslav Egorov wrote: On 2011/05/23 19:44:19, antonm wrote: > do we want to persist independence related state across MakeWeak/ClearWeakness > calls, maybe they should reset independence flags? > I think independence

[v8-dev] Re: Handle changes to the Object prototype in fast handling of arrays (issue7067019)

2011-05-24 Thread antonm
LGTM too. http://codereview.chromium.org/7067019/diff/1/src/builtins.cc File src/builtins.cc (right): http://codereview.chromium.org/7067019/diff/1/src/builtins.cc#newcode369 src/builtins.cc:369: // This method depends on non writability of Object and Array prototype Sigh, but this comment shou

[v8-dev] Re: Move young independent handles to the end of the global handle's list. (issue7062004)

2011-05-23 Thread antonm
DBC http://codereview.chromium.org/7062004/diff/1/src/global-handles.cc File src/global-handles.cc (right): http://codereview.chromium.org/7062004/diff/1/src/global-handles.cc#newcode114 src/global-handles.cc:114: void GlobalHandles::Node::ClearWeakness(GlobalHandles* global_handles) { do we wa

[v8-dev] Re: Extend Handle API with MarkIndependent. (issue7031005)

2011-05-16 Thread antonm
:29:44, Vyacheslav Egorov wrote: On 2011/05/16 15:20:44, antonm wrote: > this slightly worries me. Currently we use data read from object itself to do > some dispatches (well, maybe not in the case of WebGL arrays, but definitely for > nodes). Now I must be careful to fetch those da

[v8-dev] Re: Extend Handle API with MarkIndependent. (issue7031005)

2011-05-16 Thread antonm
14:55:34, Vyacheslav Egorov wrote: On 2011/05/16 14:13:19, antonm wrote: > I think you should reset independent flag here. It's done by result->Initialize(value) below. Am I missing some code path? http://codereview.chromium.org/7031005/diff/2003/test/cctest/test-api.cc File test/

[v8-dev] Re: Extend Handle API with MarkIndependent. (issue7031005)

2011-05-16 Thread antonm
DBC Overall, independent doesn't look like a very good name to me. Something like early collectable feels somewhat better, but YMMV. http://codereview.chromium.org/7031005/diff/1/include/v8.h File include/v8.h (right): http://codereview.chromium.org/7031005/diff/1/include/v8.h#newcode391 i

[v8-dev] Re: Introduce lazy sweeping. (issue6970004)

2011-05-10 Thread antonm
DBC http://codereview.chromium.org/6970004/diff/1/src/incremental-marking.cc File src/incremental-marking.cc (right): http://codereview.chromium.org/6970004/diff/1/src/incremental-marking.cc#newcode242 src/incremental-marking.cc:242: PrintF("[IncrementalMarking] Start sweeping.\n"); if (FLAG_tr

[v8-dev] Re: Fix debuger evaluation on a breakpoint inside eval (issue6875005)

2011-04-15 Thread antonm
LGTM, but let's wait for Søren http://codereview.chromium.org/6875005/diff/1/test/mjsunit/debug-evaluate-with.js File test/mjsunit/debug-evaluate-with.js (right): http://codereview.chromium.org/6875005/diff/1/test/mjsunit/debug-evaluate-with.js#newcode51 test/mjsunit/debug-evaluate-with.js:51:

[v8-dev] Minor cosmetic changes. (issue6875003)

2011-04-15 Thread antonm
Reviewers: Vitaly Repeshko, Description: Minor cosmetic changes. Please review this at http://codereview.chromium.org/6875003/ SVN Base: https://v8.googlecode.com/svn/branches/bleeding_edge Affected files: M src/messages.cc M test/cctest/test-api.cc Index: src/messages.cc diff --git a/s

[v8-dev] Re: Allow recursive messages reporting as it is already used. (issue6820003)

2011-04-15 Thread antonm
http://codereview.chromium.org/6875003 should address 2 of 3 your suggestions. http://codereview.chromium.org/6820003/diff/6/src/messages.cc File src/messages.cc (right): http://codereview.chromium.org/6820003/diff/6/src/messages.cc#newcode134 src/messages.cc:134: v8::TryCatch tryCatch; On 20

[v8-dev] Re: Allow recursive messages reporting as it is already used. (issue6820003)

2011-04-11 Thread antonm
thanks a lot for review, guys. Landed. http://codereview.chromium.org/6820003/diff/1/test/cctest/test-api.cc File test/cctest/test-api.cc (left): http://codereview.chromium.org/6820003/diff/1/test/cctest/test-api.cc#oldcode8680 test/cctest/test-api.cc:8680: CompileRun("throw 'ThrowInJS';"); On

[v8-dev] Allow recursive messages reporting as it is already used. (issue6820003)

2011-04-08 Thread antonm
Reviewers: Mads Ager, Vitaly Repeshko, Yury Semikhatsky, Message: Guys, may you have a look? Regression was detected by layout tests (but it wasn't for some reasonable before :( Many thanks to Yury for help! Description: Allow recursive messages reporting as it is already used. Instead disca

[v8-dev] Re: Report stack overflow exceptions to V8 message listeners (issue6816021)

2011-04-08 Thread antonm
Nice. Off you go, ol' code. LGTM. http://codereview.chromium.org/6816021/ -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev

[v8-dev] Re: Fix auto suspension of the sampler thread. (issue6801060)

2011-04-07 Thread antonm
LGTM http://codereview.chromium.org/6801060/ -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev

[v8-dev] Re: Report stack overflow exceptions to V8 message listeners (issue6816021)

2011-04-07 Thread antonm
http://codereview.chromium.org/6816021/diff/1/src/top.cc File src/top.cc (right): http://codereview.chromium.org/6816021/diff/1/src/top.cc#newcode832 src/top.cc:832: if (!thread_local_top()->pending_message_obj_->IsTheHole()) { sorry for popping up, but maybe stack overflow exception should prob

[v8-dev] Re: Debugger: show local scope before with in scope chain for functions created inside with block (issue6804015)

2011-04-06 Thread antonm
LGTM, but let's wait for Søren's comments as well. http://codereview.chromium.org/6804015/diff/1/src/runtime.cc File src/runtime.cc (right): http://codereview.chromium.org/6804015/diff/1/src/runtime.cc#newcode9825 src/runtime.cc:9825: // The context_ is a with block from the outer function. may

[v8-dev] Re: Make object groups and implicit references a bit more lightweight. (issue6800003)

2011-04-06 Thread antonm
LGTM http://codereview.chromium.org/683/diff/1/src/allocation.h File src/allocation.h (right): http://codereview.chromium.org/683/diff/1/src/allocation.h#newcode55 src/allocation.h:55: class CustomlyAllocated { I don't like the name, but I cannot come up with anything better. However,

[v8-dev] Re: Introduce v8::Object::CreationContext method. (issue6759054)

2011-04-01 Thread antonm
Thanks a lot for review, Mads. Submitting. http://codereview.chromium.org/6759054/diff/1004/include/v8.h File include/v8.h (right): http://codereview.chromium.org/6759054/diff/1004/include/v8.h#newcode1656 include/v8.h:1656: * Returns a context in which the object was created. On 2011/04/01 11

[v8-dev] Introduce v8::Object::CreationContext method. (issue6759054)

2011-04-01 Thread antonm
Reviewers: Mads Ager, Message: Mads, may you have a look? I used it for my experiments. But yesterday Aaron (cc'ed) independently showed an interest in such an API. Description: Introduce v8::Object::CreationContext method. That allows to find out a global context in which the object was

[v8-dev] Do not create a SharedFunctionInfo for closures on each recompilation. (issue6706016)

2011-04-01 Thread antonm
Reviewers: Mads Ager, Kevin Millikin, Message: Guys, may you have a look? Search algorithm is quite simple. If it'll prove to be a problem we can easily improve it emitting a separate list of emitted SFIs. Description: Do not create a SharedFunctionInfo for closures on each recompilation.

[v8-dev] Re: Fast TLS support. (issue6696112)

2011-03-25 Thread antonm
LGTM I hope that this fast way to read TLS is covered by the current tests as it's quite a common path, correct? http://codereview.chromium.org/6696112/diff/1/SConstruct File SConstruct (right): http://codereview.chromium.org/6696112/diff/1/SConstruct#newcode794 SConstruct:794: 'help': 'ena

[v8-dev] Re: Make exception thrown via v8 public API propagate to v8::TryCatch as JS thrown exceptions do. (issue6685087)

2011-03-22 Thread antonm
Thanks a lot for review, Mads and Vitaly. http://codereview.chromium.org/6685087/diff/7001/src/api.cc File src/api.cc (right): http://codereview.chromium.org/6685087/diff/7001/src/api.cc#newcode2784 src/api.cc:2784: static Local GetPropertyByLookup(i::Isolate* isolate, On 2011/03/22 07:32:14, M

[v8-dev] Re: Shorten bail out checks when calling code which can fail. (issue6713004)

2011-03-21 Thread antonm
As discussed on IM, just shorten miss branch generation sequence. http://codereview.chromium.org/6713004/ -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev

[v8-dev] Re: Make exception thrown via v8 public API propagate to v8::TryCatch as JS thrown exceptions do. (issue6685087)

2011-03-21 Thread antonm
http://codereview.chromium.org/6685087/diff/1/src/api.cc File src/api.cc (right): http://codereview.chromium.org/6685087/diff/1/src/api.cc#newcode2395 src/api.cc:2395: // propagate outside. On 2011/03/21 10:33:22, Mads Ager wrote: propagate -> to propagate? Done. http://codereview.chromium.o

[v8-dev] Make exception thrown via v8 public API propagate to v8::TryCatch as JS thrown exceptions do. (issue6685087)

2011-03-18 Thread antonm
Reviewers: Mads Ager, Vitaly Repeshko, Message: Guys, may you have a look? It is rebaselined version of http://code.google.com/p/v8/source/detail?r=7258 modulo two changes: 1) do not recurse in MessageHandler::ReportMessage (implemented with static in_progress guard): that solves the princi

[v8-dev] Re: [Isolates] Merge (7253,7263] from bleeding_edge. (issue6711034)

2011-03-18 Thread antonm
Rubberstamp LGTM http://codereview.chromium.org/6711034/ -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev

[v8-dev] Revert r7258 and r7260. (issue6709028)

2011-03-18 Thread antonm
Reviewers: Vitaly Repeshko, Description: Revert r7258 and r7260. They apparently break Threading tests on at least Mac and Win64. TBR=vita...@chromium.org Committed: http://code.google.com/p/v8/source/detail?r=7262 Please review this at http://codereview.chromium.org/6709028/ SVN Base: https

[v8-dev] Fix build. (issue6688032)

2011-03-18 Thread antonm
Reviewers: Vitaly Repeshko, Message: I'll investigate why it hadn't been caught by the tests. Description: Fix build. TBR=vita...@chromium.org Please review this at http://codereview.chromium.org/6688032/ SVN Base: https://v8.googlecode.com/svn/branches/bleeding_edge Affected files: M src

[v8-dev] Re: [Isolates] Small fixes after the last merge. (issue6709026)

2011-03-18 Thread antonm
LGTM http://codereview.chromium.org/6709026/ -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev

[v8-dev] Shorten bail out checks when calling code which can fail. (issue6713004)

2011-03-17 Thread antonm
Reviewers: Mads Ager, Message: Mads, stupid cleanup change. I am not sure macro is that good---people will forget to use it soon. But I think we should shorten GenerateMissBranch sequences at least. I'll go over other files if you think overall approach is ok. Description: Shorten bail out ch

[v8-dev] Re: Refactor fast API call. (issue6686003)

2011-03-17 Thread antonm
, antonm wrote: > On 2011/03/14 15:33:10, Vitaly Repeshko wrote: > > Should work with "const CallOptimization&". > > Well, it looks like typically v8 uses pointers, not references. I'll switch if > you insist. I insist on every comment I write, unless marked ot

[v8-dev] Re: Fix Array::New(length) in the API to return an array with the provided length. (issue6674034)

2011-03-16 Thread antonm
LGTM. Maybe rename Factory::NewJSArray argument to capacity instead of length in this change as well, but up to you http://codereview.chromium.org/6674034/ -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev

[v8-dev] Re: Introduce one way dependencies into object grouping. (issue6686053)

2011-03-15 Thread antonm
Thanks a lot for review, Vitaly. I've added a test with a cycle. Regarding the name. I am not insisting on my name (although I think it's indeed good enough). I would only ask for something with a proper plural (I started with AddImplicitReferences, but it's hard to make GlobalHandles::Im

[v8-dev] Re: Introduce one way dependencies into object grouping. (issue6686053)

2011-03-15 Thread antonm
In offline discussion Vitaly and Misha suggested to switch to the different API. Thanks a lot, guys. May you have a look? http://codereview.chromium.org/6686053/ -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev

[v8-dev] Re: Refactor fast API call. (issue6686003)

2011-03-15 Thread antonm
http://codereview.chromium.org/6686003/diff/1/src/hydrogen.cc File src/hydrogen.cc (right): http://codereview.chromium.org/6686003/diff/1/src/hydrogen.cc#newcode4265 src/hydrogen.cc:4265: // because it is likely to generate better code. Similarly, we On 2011/03/14 15:33:10, Vitaly Repeshko wrote

[v8-dev] Re: Introduce one way dependencies into object grouping. (issue6686053)

2011-03-14 Thread antonm
Sure, may you have another look? I am not introducing structures for handles' collection (length, array and your info object) yet, let's reserve it for a separate CL. On 2011/03/14 18:33:07, Mikhail Naganov (Chromium) wrote: On 2011/03/14 18:01:39, antonm wrote: > Guys, >

[v8-dev] Introduce one way dependencies into object grouping. (issue6686053)

2011-03-14 Thread antonm
Reviewers: Mads Ager, Mikhail Naganov (Chromium), Vitaly Repeshko, Message: Guys, may you have a look? I am esp. uneasy about all those testing utils in liveobjects and profiler. Description: Introduce one way dependencies into object grouping. Those are necessary to properly manage relations

[v8-dev] Refactor fast API call. (issue6686003)

2011-03-11 Thread antonm
Reviewers: Vitaly Repeshko, Message: Vitaly, may you have a look? I think this unification is a good idea, but I by no means insist. If you're fine with general approach, I'll finish ports to other platforms. Description: Refactor fast API call. Make it use custom call generator infrastru

[v8-dev] Faster invocation for most of API calls. (issue6672026)

2011-03-11 Thread antonm
Reviewers: Kevin Millikin, Vitaly Repeshko, Message: Guys, may you have a look? I am going to have a quick look if we can exploit custom call generators to achieve the same effect. I am not sure that performance gains of inlining API call are balanced by the complexity of implementation, bu

[v8-dev] Re: [Isolates] Add debugging code to ensure consistent Isolate class layout. (issue6628036)

2011-03-05 Thread antonm
LGTM http://codereview.chromium.org/6628036/ -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev

[v8-dev] Re: Handled return-value of SetElement in some cases, or avoided it in other. (issue6588130)

2011-03-02 Thread antonm
Drive-by comments http://codereview.chromium.org/6588130/diff/1/src/runtime.cc File src/runtime.cc (right): http://codereview.chromium.org/6588130/diff/1/src/runtime.cc#newcode1473 src/runtime.cc:1473: if (result.is_null()) return Failure::Exception(); you may use RETURN_IF_EMPTY_HANDLE http:/

[v8-dev] Landing for Zaheer. (issue6576035)

2011-02-24 Thread antonm
Reviewers: Erik Corry, SeRya, Zaheer, Message: TBRing to you, guys. Description: Landing for Zaheer. Direct call accessor getter callbacks (arm implementation). Original review: http://codereview.chromium.org/6462029/ Please review this at http://codereview.chromium.org/6576035/ SVN Base: ht

[v8-dev] Get property may throw an exception thanks to JS accessors. (issue6580030)

2011-02-24 Thread antonm
Reviewers: Mads Ager, mmaly, Message: Mads and Martin, tiny review for you. Description: Get property may throw an exception thanks to JS accessors. Check result before and bail out if exception has been thrown. BUG=v8:1172 TEST=test/mjsunit/regress/regress-1172-bis.js Please review this at

[v8-dev] Re: Direct call accessor getter callbacks (arm implementation). (issue6462029)

2011-02-24 Thread antonm
Thanks a lot, Zaheer. Landing it. http://codereview.chromium.org/6462029/ -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev

[v8-dev] Properly reset external catcher if exception couldn't be externally caught. (issue6538081)

2011-02-21 Thread antonm
Reviewers: Mads Ager, Message: Mads, another exception handling related review for you. Description: Properly reset external catcher if exception couldn't be externally caught. We can wrongly assume that exception which is not intended to be caught by external try/catch should be caught if thi

[v8-dev] Re: Minor refactoring: unify lazy function compilation for in loop and no in loop variants. (issue6542017)

2011-02-21 Thread antonm
Thanks a lot for review and suggestions, guys. Submitting http://codereview.chromium.org/6542017/diff/1/src/handles.cc File src/handles.cc (right): http://codereview.chromium.org/6542017/diff/1/src/handles.cc#newcode859 src/handles.cc:859: return CompileLazyFunction(function, flag, false /* in

[v8-dev] Re: Direct call accessor getter callbacks (arm implementation). (issue6462029)

2011-02-21 Thread antonm
v8::Handle (*SimulatorRuntimeDirectApiCall)(int32_t arg0); On 2011/02/21 10:25:35, Zaheer wrote: On 2011/02/17 16:13:44, antonm wrote: > Do we need word Direct here? It looks like simulator doesn't have to know it's > anything direct. But if you feel it conveys important semant

[v8-dev] Minor refactoring: unify lazy function compilation for in loop and no in loop variants. (issue6542017)

2011-02-18 Thread antonm
Reviewers: Kevin Millikin, Message: Kevin, another tiny review for you Description: Minor refactoring: unify lazy function compilation for in loop and no in loop variants. Please review this at http://codereview.chromium.org/6542017/ SVN Base: https://v8.googlecode.com/svn/branches/bleedin

[v8-dev] Minor cleanup. (issue6541020)

2011-02-18 Thread antonm
Reviewers: Kevin Millikin, Message: Kevin, tiny review for you. Description: Minor cleanup. Do not relookup code object and use optimized_code instead shadowing existing code local. Please review this at http://codereview.chromium.org/6541020/ SVN Base: https://v8.googlecode.com/svn/branches

[v8-dev] Re: Use [[DefineOwnProperty]] to put 'constructor' field on the protoype object. (issue6531037)

2011-02-18 Thread antonm
http://codereview.chromium.org/6531037/diff/1/src/runtime.cc File src/runtime.cc (right): http://codereview.chromium.org/6531037/diff/1/src/runtime.cc#newcode6930 src/runtime.cc:6930: RETURN_IF_EMPTY_HANDLE(result); On 2011/02/17 20:55:41, Mads Ager wrote: Shouldn't we be able to create a test

[v8-dev] Re: Add access checks to Object.preventExtensions + add regression test for 1027.... (issue6534019)

2011-02-18 Thread antonm
LGTM http://codereview.chromium.org/6534019/diff/7004/test/cctest/test-api.cc File test/cctest/test-api.cc (right): http://codereview.chromium.org/6534019/diff/7004/test/cctest/test-api.cc#newcode5691 test/cctest/test-api.cc:5691: // Regression test ofr issue 1171. nit: ofr => for http://coder

[v8-dev] Use [[DefineOwnProperty]] to put 'constructor' field on the protoype object. (issue6531037)

2011-02-17 Thread antonm
Reviewers: Mads Ager, Message: Mads, may you have a look? Description: Use [[DefineOwnProperty]] to put 'constructor' field on the protoype object. That better follows ECMA-262 (see 13.2 Creating Function Objects) and allows to ignore nasty JS accessors for 'constructor' property. BUG=v8:1172

[v8-dev] Make OutOfMemory exception thrown from JS call into FatalProcessOutOfMemory as well. (issue6528050)

2011-02-17 Thread antonm
Reviewers: Mads Ager, Message: Mads, may you have a look? Description: Make OutOfMemory exception thrown from JS call into FatalProcessOutOfMemory as well. That unifies the behaviour with CALL_HEAP_FUNCTION macro. BUG=v8:1165 Please review this at http://codereview.chromium.org/6528050/

[v8-dev] Re: Direct call accessor getter callbacks (arm implementation). (issue6462029)

2011-02-17 Thread antonm
Thanks a lot for working on this! http://codereview.chromium.org/6462029/diff/1/src/arm/simulator-arm.cc File src/arm/simulator-arm.cc (right): http://codereview.chromium.org/6462029/diff/1/src/arm/simulator-arm.cc#newcode1534 src/arm/simulator-arm.cc:1534: typedef v8::Handle (*SimulatorRuntime

[v8-dev] Re: Add access checks to Object.preventExtensions + add regression test for 1027.... (issue6534019)

2011-02-17 Thread antonm
Almost LGTM, one question though http://codereview.chromium.org/6534019/diff/1/src/runtime.cc File src/runtime.cc (right): http://codereview.chromium.org/6534019/diff/1/src/runtime.cc#newcode886 src/runtime.cc:886: // If access checks fail simply return false nit: dot at the end, please http:/

[v8-dev] Re: Properly process try/finally blocks. (issue6526016)

2011-02-16 Thread antonm
Thanks a lot for review, Mads. Submitting. http://codereview.chromium.org/6526016/diff/1006/src/top.cc File src/top.cc (right): http://codereview.chromium.org/6526016/diff/1006/src/top.cc#newcode938 src/top.cc:938: // When throwing the exception, we found no or another v8::TryCatch On 2011/02/

[v8-dev] Properly process try/finally blocks. (issue6526016)

2011-02-15 Thread antonm
Reviewers: Mads Ager, Message: Mads, may you have a look? I was considering refactoring and renaming ShouldReportException (make explicit a part which traverses the stack vs. verbosity), but decided not to do right now (just lazy) can_be_exteranlly_caught might be better spelled as may_be

[v8-dev] Introduce new runtime function to make join with lower memory usage. (issue6520004)

2011-02-14 Thread antonm
Reviewers: Mads Ager, sandholm, Message: Anders and Mads, may you have a look? Description: Introduce new runtime function to make join with lower memory usage. Do not use generic StringBuilderConcat which requires array passed to keep both elements and separator (which roughly double size of

[v8-dev] Re: Properly treat exceptions thrown while compiling. (issue6487021)

2011-02-11 Thread antonm
Thanks a lot for review, Kevin. I also raised the constant slightly to allow test to run under x64 and disabled it for ARM. Submitting http://codereview.chromium.org/6487021/diff/4001/src/flag-definitions.h File src/flag-definitions.h (right): http://codereview.chromium.org/6487021/diff/40

[v8-dev] Re: Properly treat exceptions thrown while compiling. (issue6487021)

2011-02-11 Thread antonm
Kevin, may you have another quick look? I've added flag to restrict v8 stack region size and that makes test pass ways faster. Running test on other platforms, fingers-crossedly. yours, anton. On 2011/02/11 11:43:18, Kevin Millikin wrote: Change LGTM. Go ahead and commit everything as is,

[v8-dev] Properly treat exceptions thrown while compiling. (issue6487021)

2011-02-11 Thread antonm
Reviewers: Kevin Millikin, Message: Kevin, may you have a look, please? I am not best versed in this part of the system, so there might be better way to attack the problem---exception thrown in ParseLazy. For example, it might be not the best idea to disable optimization, although it at th

[v8-dev] Re: Bypass JS accessors when building error array. (issue6481001)

2011-02-10 Thread antonm
Thanks a lot for review, Lasse http://codereview.chromium.org/6481001/diff/1/test/mjsunit/regress/regress-1130.js File test/mjsunit/regress/regress-1130.js (right): http://codereview.chromium.org/6481001/diff/1/test/mjsunit/regress/regress-1130.js#newcode35 test/mjsunit/regress/regress-1130.js:

[v8-dev] Re: Fix various places which do not check if SetProperty threw an exception. (issue6480003)

2011-02-10 Thread antonm
Thanks for review. Submitting http://codereview.chromium.org/6480003/diff/2001/src/factory.cc File src/factory.cc (right): http://codereview.chromium.org/6480003/diff/2001/src/factory.cc#newcode583 src/factory.cc:583: // Currently save as is only invoked from Genesis. On 2011/02/10 10:26:43, M

[v8-dev] Fix various places which do not check if SetProperty threw an exception. (issue6480003)

2011-02-09 Thread antonm
Reviewers: Mads Ager, Message: Mads, may you have a look? Description: Fix various places which do not check if SetProperty threw an exception. Please review this at http://codereview.chromium.org/6480003/ SVN Base: https://v8.googlecode.com/svn/branches/bleeding_edge Affected files: M src

[v8-dev] Bypass JS accessors when building error array. (issue6481001)

2011-02-09 Thread antonm
Reviewers: Lasse Reichstein, Message: Lasse, may you have a look? Description: Bypass JS accessors when building error array. In the presence of JS accessors for elements on Object.prototype JSArray::SetFastElement may throw or its behaviour can be altered. Instead operate on plain FixedArr

[v8-dev] Fix forging of object's identity hashes. (issue6472001)

2011-02-09 Thread antonm
Reviewers: Mads Ager, Message: Mads, a review for you. Description: Fix forging of object's identity hashes. Do not do standard property lookup on hidden properties object as it might reach Object.prototype which can be altered to forge identity hashes. Instead do only local lookup. Please re

[v8-dev] Fix forging of object's identity hashes. (issue6472001)

2011-02-09 Thread antonm
Reviewers: Mads Ager, Message: Mads, a review for you. Description: Fix forging of object's identity hashes. Do not do standard property lookup on hidden properties object as it might reach Object.prototype which can be altered to forge identity hashes. Instead do only local lookup. Please re

[v8-dev] Reapply http://code.google.com/p/v8/source/detail?r=6555 (issue6461028)

2011-02-09 Thread antonm
Reviewers: Rico, Message: TBRing to you, Rico. Description: Reapply http://code.google.com/p/v8/source/detail?r=6555 Compare JSObjects by identity immediately. When invoking EQUALS JS builtin, 1st argument is passed as a receiver and if it's a global object, it gets overwritten with global pro

[v8-dev] Re: Merge a number of assertion failure fixes to the 3.0 branch. (issue6461022)

2011-02-09 Thread antonm
My part LGTM http://codereview.chromium.org/6461022/diff/5007/test/mjsunit/regress/regress-1103.js File test/mjsunit/regress/regress-1103.js (right): http://codereview.chromium.org/6461022/diff/5007/test/mjsunit/regress/regress-1103.js#newcode32 test/mjsunit/regress/regress-1103.js:32: obj = Ob

[v8-dev] Do not invoke any setters when forming stack trace JS object. (issue6463022)

2011-02-09 Thread antonm
Reviewers: Mads Ager, Description: Do not invoke any setters when forming stack trace JS object. Please review this at http://codereview.chromium.org/6463022/ SVN Base: https://v8.googlecode.com/svn/branches/bleeding_edge Affected files: M src/top.cc M test/cctest/test-api.cc Index: src/

[v8-dev] Re: Use GC-safe version when setting elements. (issue6463001)

2011-02-09 Thread antonm
http://codereview.chromium.org/6463001/diff/1/src/runtime.cc File src/runtime.cc (right): http://codereview.chromium.org/6463001/diff/1/src/runtime.cc#newcode7394 src/runtime.cc:7394: Handle::cast(holder)->set(index, *value); Agree, but in this case I am afraid we cannot use SetElement as a cont

[v8-dev] Use GC-safe version when setting elements. (issue6463001)

2011-02-08 Thread antonm
Reviewers: Mads Ager, Message: Mads, may you have a look? Description: Use GC-safe version when setting elements. BUG=1125 TEST=test/mjsunit/regress/regress-1125.js Please review this at http://codereview.chromium.org/6463001/ SVN Base: https://v8.googlecode.com/svn/branches/bleeding_edge A

[v8-dev] Re: Port fix for duplicate AST ID for deoptimization to ARM and x64. (issue6458001)

2011-02-08 Thread antonm
LGTM http://codereview.chromium.org/6458001/ -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev

[v8-dev] Re: Propagate exceptions thrown when setting elements. (issue6451004)

2011-02-08 Thread antonm
Submitting http://codereview.chromium.org/6451004/diff/6002/src/objects.cc File src/objects.cc (right): http://codereview.chromium.org/6451004/diff/6002/src/objects.cc#newcode1728 src/objects.cc:1728: return Heap::the_hole_value(); On 2011/02/08 18:59:11, Mads Ager wrote: Can we do an output p

[v8-dev] Re: Fix wrong assumption in parser that parsing a function literal cannot throw an exception. (issue6453009)

2011-02-08 Thread antonm
LGTM http://codereview.chromium.org/6453009/ -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev

[v8-dev] 1) Return failure if any of property sets failed; (issue6454011)

2011-02-08 Thread antonm
Reviewers: Mads Ager, Message: Mads, may you have a look? I've added three bailout checks, but managed to test only the last one. I can hit the path of first check, but cannot make it fail. And I cannot hit the path of the second check. In any event, those bailouts shouldn't hurt (me t

[v8-dev] We cannot assert that v8 is running in fatal error callback. (issue6450005)

2011-02-08 Thread antonm
Reviewers: Mads Ager, Søren Gjesse, Message: Guys, may you have a look? Description: We cannot assert that v8 is running in fatal error callback. BUG=v8: Please review this at http://codereview.chromium.org/6450005/ SVN Base: https://v8.googlecode.com/svn/branches/bleeding_edge Affected

[v8-dev] Check if Array.prototype.__proto__ has been reset to null. (issue6454004)

2011-02-08 Thread antonm
Reviewers: Rico, Message: Rico, may you have a look? Description: Check if Array.prototype.__proto__ has been reset to null. BUG=v8:1121 TEST=test/mjsunit/regress/regress-1121.js Please review this at http://codereview.chromium.org/6454004/ SVN Base: https://v8.googlecode.com/svn/branches/bl

[v8-dev] Do sanity check of exception state when returning from native to JS. (issue6450004)

2011-02-08 Thread antonm
Reviewers: Mads Ager, Lasse Reichstein, Message: Lasse and Mads, another tiny review. If you're fine with it, I'll bring to other ports. Description: Do sanity check of exception state when returning from native to JS. If --debug-code is on, check that returned value and Top::has_pending_ex

[v8-dev] Re: Propagate exceptions thrown when setting elements. (issue6451004)

2011-02-08 Thread antonm
Thanks a lot for comments. http://codereview.chromium.org/6451004/diff/1/src/messages.cc File src/messages.cc (right): http://codereview.chromium.org/6451004/diff/1/src/messages.cc#newcode78 src/messages.cc:78: Factory::NewJSArrayWithElements(arguments_elements); On 2011/02/08 14:17:56, Lasse R

[v8-dev] Propagate exceptions thrown when setting elements. (issue6451004)

2011-02-08 Thread antonm
Reviewers: Mads Ager, Lasse Reichstein, Message: Lasse and Mads, may you have a look? And I'll send enough path which adds checks in CEntry stub what return value and Top::pending_exception are consistent on coming back to JS code. Description: Propagate exceptions thrown when setting eleme

[v8-dev] Re: Correct propagation of exceptions from setters. (issue6451003)

2011-02-08 Thread antonm
LGTM http://codereview.chromium.org/6451003/diff/1/src/runtime.cc File src/runtime.cc (right): http://codereview.chromium.org/6451003/diff/1/src/runtime.cc#newcode1093 src/runtime.cc:1093: if (result.is_null()) return Failure::Exception(); I know we do not typically do it, so feel free to ignor

[v8-dev] Merge r6592, r6612, r6618, r6626 and r6636 into 3.0 branch (issue6334107)

2011-02-04 Thread antonm
Reviewers: Mads Ager, Rico, Message: TBRing to you Description: Merge r6592, r6612, r6618, r6626 and r6636 into 3.0 branch Please review this at http://codereview.chromium.org/6334107/ SVN Base: http://v8.googlecode.com/svn/branches/3.0/ Affected files: M src/objects.cc M src/runt

[v8-dev] Re: Landing for Zaheer Ahmad. (issue6286078)

2011-02-04 Thread antonm
All comments except for one are addressed, http://codereview.chromium.org/6286078/diff/1/AUTHORS File AUTHORS (right): http://codereview.chromium.org/6286078/diff/1/AUTHORS#newcode38 AUTHORS:38: Mike Gilbert On 2011/02/04 09:55:55, Erik Corry wrote: Perhaps you could move Mike who is not in a

[v8-dev] Re: Merge r6592, r6612, r6618 and r6626 into 2.5 branch. (issue6411001)

2011-02-04 Thread antonm
Now with r6636. http://codereview.chromium.org/6411001/ -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev

[v8-dev] Re: Merge r6592, r6612, r6618 and r6626 into 2.4 branch (issue6250133)

2011-02-04 Thread antonm
Now with r6636. http://codereview.chromium.org/6250133/ -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev

  1   2   3   4   5   6   7   >