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

RESOLVED FIXED in mozilla35

Status

()

Core
JavaScript Engine
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: jimb, Assigned: jimb)

Tracking

unspecified
mozilla35
Points:
---
Bug Flags:
in-testsuite -
qe-verify -

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

3 years ago
Created attachment 8498187 [details] [diff] [review]
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.
Attachment #8498187 - Flags: review?(luke)

Comment 1

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

lol
Attachment #8498187 - Flags: review?(luke) → review+

Comment 2

3 years ago
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.
(Assignee)

Comment 3

3 years ago
(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.
(Assignee)

Updated

3 years ago
Assignee: nobody → jimb
Flags: in-testsuite-
Target Milestone: --- → mozilla35
https://hg.mozilla.org/mozilla-central/rev/c70ca7e54741
Status: NEW → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
Flags: qe-verify-
You need to log in before you can comment on or make changes to this bug.