Thanks, Kevin and Lasse! Your comments are really good.
http://codereview.chromium.org/67168/diff/1/2 File src/func-name-inferrer.cc (right): http://codereview.chromium.org/67168/diff/1/2#newcode68 Line 68: return; On 2009/04/16 08:06:58, Kevin Millikin wrote: > MaybeInferFunctionName is only called from InferAndLeave, which is inlined. It > might make sense to move the check and early bailout into InferAndLeave. Done. I also renamed it to InferFunctionsNames. http://codereview.chromium.org/67168/diff/1/2#newcode74 Line 74: funcs_to_infer_.Clear(); On 2009/04/16 08:06:58, Kevin Millikin wrote: > Clearing the list deletes the backing buffer, which then gets reallocated on the > first subsequent insertion and resized on the second. You might consider > changing the loop to 'while (!funcs_to_infer_.is_empty()) ...' and use > List::RemoveLast and FuncNameInferrer::set_inferred_name in the body. I followed Lasse's advice. http://codereview.chromium.org/67168/diff/1/2#newcode74 Line 74: funcs_to_infer_.Clear(); On 2009/04/16 08:31:23, Lasse Reichstein wrote: > Alternatively you can use funcs_to_infer_.Rewind(0) (which is basically an oddly > named "SetLength") to clear the list without deleting the backing buffer. Done. http://codereview.chromium.org/67168/diff/1/3 File src/func-name-inferrer.h (right): http://codereview.chromium.org/67168/diff/1/3#newcode60 Line 60: void Leave() { On 2009/04/16 08:06:58, Kevin Millikin wrote: > I don't think Leave is every called explicitly (only by InferAndLeave?), so you > might just inline it there and remove it from the API. Done. http://codereview.chromium.org/67168/diff/1/4 File src/rewriter.cc (right): http://codereview.chromium.org/67168/diff/1/4#newcode285 Line 285: scoped_fni.Enter(); On 2009/04/16 08:06:58, Kevin Millikin wrote: > There is also a call to ScopedFuncNameInferrer::Enter in > AstOptimizer::VisitCallRuntime that is currently guarded by checking that the > expression is a function literal. Do you want to allow other expression types > there as well? Um, that's tricky, unfortunately. Consider the example: var v = f(function() {}) Would I remove the check from VisitCallRuntime, this will erroneously infer a name to the function passed as a parameter. Of course, leaving the check disables inferring a name in this case: (in the global scope) var f = 0 ? function() {} : function() {} It seems that in order to pass both tests, I need to add another state into the FuncNameInferrer, that will be entered upon visiting the CALL AST node. But as this all seems quite exotic, I'll better leave it as is for now. http://codereview.chromium.org/67168 --~--~---------~--~----~------------~-------~--~----~ v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev -~----------~----~----~----~------~----~------~--~---
