Closed
Bug 1257077
Opened 8 years ago
Closed 8 years ago
Implement js::Fixed{Invoke,Construct}Args and js::Any{Invoke,Construct}Args as invoke/construct argument base classes
Categories
(Core :: JavaScript Engine, defect)
Core
JavaScript Engine
Tracking
()
RESOLVED
FIXED
mozilla48
Tracking | Status | |
---|---|---|
firefox48 | --- | fixed |
People
(Reporter: Waldo, Assigned: Waldo)
Details
Attachments
(1 file)
11.60 KB,
patch
|
efaust
:
review+
|
Details | Diff | Splinter Review |
Bunches of call/construct instances in C++ use a fixed number of arguments. There's no reason they should be paying GCVector allocation costs. Add some extra classes to support this, plus a couple more to serve as base classes for all invoke args, and for all construct args.
Assignee | ||
Comment 1•8 years ago
|
||
Attachment #8731046 -
Flags: review?(efaustbmo)
Comment 2•8 years ago
|
||
Comment on attachment 8731046 [details] [diff] [review] Patch Review of attachment 8731046 [details] [diff] [review]: ----------------------------------------------------------------- Nice. ::: js/src/vm/Interpreter.cpp @@ +599,5 @@ > MutableHandleObject objp) > { > + // Explicitly qualify to bypass AnyConstructArgs's deliberate shadowing. > + args.CallArgs::setCallee(fval); > + args.CallArgs::setThis(MagicValue(JS_IS_CONSTRUCTING)); Yeah, this is a little ugly, but I understand why delete is important here. I wonder if we will have to recant on this, for the rest of browser ergonomics. If not, great! ::: js/src/vm/Stack.h @@ +955,4 @@ > > + public: > + bool init(unsigned argc) { > + MOZ_ASSERT(2 + argc + Construct > argc); // no overflow Not your fault. I actually changed this assert last, I think, but can we add a comment that's like // Callee, This, Args, [new.target if constructing] or some such? 2 is dotted around, but looks pretty magic, here. @@ +991,5 @@ > }; > > +/** Function call args of statically-known count. */ > +template <size_t N> > +class FixedInvokeArgs : public detail::FixedArgsBase<false, N> can we use MaybeConstruct, or something similar here, instead of hardcoding true and false? Seems like CONSTRUCT is clearer than true.
Attachment #8731046 -
Flags: review?(efaustbmo) → review+
Comment 4•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/4029d2aeb270
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
status-firefox48:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
You need to log in
before you can comment on or make changes to this bug.
Description
•