After carrying out this change, it now seems silly not just to combine the three functions into one, and to use it as the body of GenerateLoadInterceptor. That removes all this infrastructure, and I think it can be as clear or clearer than the current solution. I'll give it a try.
On Wed, May 26, 2010 at 6:19 PM, <[email protected]> wrote: > LGTM. > > Of comments the only one I am feeling strongly for is a name for > interceptor_holder, but even in this case I'd yield. > > > http://codereview.chromium.org/2251003/diff/1/2 > File src/ia32/stub-cache-ia32.cc (right): > > http://codereview.chromium.org/2251003/diff/1/2#newcode328 > src/ia32/stub-cache-ia32.cc:328: void Compile() { > given compile doesn't accept any parameters, maybe it's time to turn it > inot a function? > > http://codereview.chromium.org/2251003/diff/1/2#newcode329 > src/ia32/stub-cache-ia32.cc:329: ASSERT(holder_->HasNamedInterceptor()); > I'd rather keep interceptor_holder name---I think it easier to > distinguish between two holders---of interceptor and of cached property. > > http://codereview.chromium.org/2251003/diff/1/2#newcode338 > src/ia32/stub-cache-ia32.cc:338: > stub_compiler_->CheckPrototypes(object_, receiver_, holder_, > maybe you could move this line into Compile* methods---it'd be easier to > track register allocation. > > http://codereview.chromium.org/2251003/diff/1/2#newcode345 > src/ia32/stub-cache-ia32.cc:345: lookup_->IsCacheable() && > I think optimize variable is more readable than long condition, but > YMMV. > > > http://codereview.chromium.org/2251003/show > -- William Hesse Software Engineer [email protected] Google Denmark ApS Frederiksborggade 20B, 1 sal 1360 København K Denmark CVR nr. 28 86 69 84 If you received this communication by mistake, please don't forward it to anyone else (it may contain confidential or privileged information), please erase all copies of it, including all attachments, and please let the sender know it went to the wrong person. Thanks. -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
