1525663 - Use generic lambdas to simplify code and remove use of Functor classes

Status: RESOLVED FIXED

(Core :: JavaScript: GC, enhancement, P3)

Description

[Jon Coppeard (:jonco)]

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.

Comment 1

[Jon Coppeard (:jonco)]

Attachment: bug1525663-use-generic-lambdas

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.

Comment 2

[Jon Coppeard (:jonco)]

Attachment: bug1525663-use-return-type-deduction

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

Comment 3

[Steve Fink [:sfink] [:s:]]

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

[Steve Fink [:sfink] [:s:]]

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

[Jeff Walden [:Waldo]]

(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.

Comment 6

[Jon Coppeard (:jonco)]

(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.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
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

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/0ae4d42ca65b
https://hg.mozilla.org/mozilla-central/rev/fdbb8bed650a