Closed Bug 1525663 Opened 8 months ago Closed 7 months ago

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

Categories

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

enhancement

Tracking

()

RESOLVED FIXED
mozilla67
Tracking Status
firefox67 --- fixed

People

(Reporter: jonco, Assigned: jonco)

References

Details

Attachments

(2 files)

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.

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: nobody → jcoppeard
Attachment #9042112 - Flags: review?(sphink)

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

Attachment #9042115 - Flags: review?(sphink)
Priority: -- → P3
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)?)
Attachment #9042112 - Flags: review?(sphink) → review+
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.
Attachment #9042115 - Flags: review?(sphink) → review+

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

(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
Status: NEW → RESOLVED
Closed: 7 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla67
Depends on: 1526751
You need to log in before you can comment on or make changes to this bug.