1075564 - I'm sorry I didn't mark my implicit constructor; may I please have it back?

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Jim Blandy :jimb]

Attachment: Give JS::ubi::Node its implicit constructor back, with the right annotation this time.

JS::ubi::Node should have an implicit constructor from JS::Value.

In bug 960786, I landed JS::ubi::Node, with an implicit Node(JS::Value) constructor.

In bug 1044596, Ehsan marked JS::ubi::Node::Node(JS::Value) explicit.

In bug 1063247, I made it implicit again.

In bug 1068024, Ehsan marked it as explicit again.

I now understand that implicit constructors should be marked MOZ_IMPLICIT. I approve of this policy; regret having not noticed my transgression earlier; and have rectified my code accordingly.

Comment 1

[Luke Wagner [:luke]]

Comment on attachment 8498187 [details] [diff] [review]
Give JS::ubi::Node its implicit constructor back, with the right annotation this time.

lol

Comment 2

[(no longer active)]

Comment on attachment 8498187 [details] [diff] [review]
Give JS::ubi::Node its implicit constructor back, with the right annotation this time.

Review of attachment 8498187 [details] [diff] [review]:
-----------------------------------------------------------------

::: js/public/UbiNode.h
@@ +276,5 @@
>      // Constructors accepting SpiderMonkey's other generic-pointer-ish types.
> +    // Note that we *do* want an implicit constructor here: JS::Value and
> +    // JS::ubi::Node are both essentially tagged references to other sorts of
> +    // objects, so letting conversions happen automatically is appropriate.
> +    Node(JS::HandleValue value) MOZ_IMPLICIT;

Nit: please move MOZ_IMPLICIT to the beginning of the declaration.

Comment 3

[Jim Blandy :jimb]

(In reply to :Ehsan Akhgari (not reading bugmail, needinfo? me!) from comment #2)
> Nit: please move MOZ_IMPLICIT to the beginning of the declaration.

Done, thanks.

Comment 4

[Jim Blandy :jimb]

https://hg.mozilla.org/integration/mozilla-inbound/rev/c70ca7e54741

Comment 5

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/c70ca7e54741