Use generic lambdas to simplify code and remove use of Functor classes
Categories
(Core :: JavaScript: GC, enhancement, P3)
Tracking
()
Tracking | Status | |
---|---|---|
firefox67 | --- | fixed |
People
(Reporter: jonco, Assigned: jonco)
References
Details
Attachments
(2 files)
13.59 KB,
patch
|
sfink
:
review+
|
Details | Diff | Splinter Review |
13.65 KB,
patch
|
sfink
:
review+
|
Details | Diff | Splinter Review |
Now that this feature is available for use we can simplify some of the code around uses of CrossCompartmentKey, and with a little more work for DispatchTyped too.
Assignee | ||
Comment 1•5 years ago
|
||
Patch to replace explicit functor classes with generic lambdas, at least as far as calling CrossCompartmentKey methods goes.
DispatchTyped turns out to be harder because there's the additional problem of what to do when unwrapping something that doesn't turn out to be a GC pointer. At the moment this is handed by giving the functor class an additional defaultValue() method via a base class.
Assignee | ||
Comment 2•5 years ago
|
||
Also return type deduction is now allowed, with the qualification "but only in template code when you would have used decltype(complex-expression)". It turns out we do a bunch of this.
This patch removes these decltype expressions that are used in CrossCompartmentKey and other places in the engine.
I couldn't get this to work for the two instances in InvokeMemberFunction so I left these:
https://searchfox.org/mozilla-central/source/js/src/frontend/EitherParser.h#34
Assignee | ||
Updated•5 years ago
|
Comment 3•5 years ago
|
||
Comment on attachment 9042112 [details] [diff] [review] bug1525663-use-generic-lambdas Review of attachment 9042112 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/src/gc/Tracer.cpp @@ +139,5 @@ > for (Compartment::WrapperEnum e(comp); !e.empty(); e.popFront()) { > mozilla::DebugOnly<const CrossCompartmentKey> prior = e.front().key(); > + e.front().mutableKey().applyToWrapped([trc, &compartments](auto tp) { > + Compartment* comp = (*tp)->maybeCompartment(); > + if (comp && compartments.has(comp)) { Why did this become conditional? (Could it be MOZ_ASSERT(comp)?)
Comment 4•5 years ago
|
||
Comment on attachment 9042115 [details] [diff] [review] bug1525663-use-return-type-deduction Review of attachment 9042115 [details] [diff] [review]: ----------------------------------------------------------------- Wow. I would not have guessed that this would show up this much.
Comment 5•5 years ago
|
||
(In reply to Jon Coppeard (:jonco) from comment #2)
I couldn't get this to work for the two instances in InvokeMemberFunction so I left these:
https://searchfox.org/mozilla-central/source/js/src/frontend/EitherParser.h#34
Yeah, deduction was I think allowed when I wrote those, but it wasn't compiling everywhere so we got sausage.
Assignee | ||
Comment 6•5 years ago
|
||
(In reply to Steve Fink [:sfink] [:s:] from comment #3)
Why did this become conditional? (Could it be MOZ_ASSERT(comp)?)
This handles the string case that was previously handled by having an extra overload in the functor class.
Pushed by jcoppeard@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/0ae4d42ca65b Make use of generic lambdas when dealing with CrossCompartmentKey variants r=sfink https://hg.mozilla.org/integration/mozilla-inbound/rev/fdbb8bed650a Use return type deduction to remove complex decltype expressions r=sfink
Comment 8•5 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/0ae4d42ca65b
https://hg.mozilla.org/mozilla-central/rev/fdbb8bed650a
Description
•