Closed Bug 1296760 Opened 8 years ago Closed 8 years ago

Some(nullptr) should coerce freely into Maybe<T*>

Categories

(Core :: MFBT, defect)

50 Branch
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla51
Tracking Status
firefox51 --- fixed

People

(Reporter: jgilbert, Assigned: Waldo)

Details

Attachments

(2 files)

      No description provided.
It seems better to me to avoid random intermediate classes here, if we can help it.  SomeNull is such a class.  Doing this by implicit conversions (which, um, I should probably check before landing this, to see whether I need any MOZ_IMPLICIT anywhere here) seems preferable -- and lets us handle the closely-related case of derived-to-base pointer conversion.
Attachment #8783115 - Flags: review?(nfroyd)
Assignee: jgilbert → jwalden+bmo
Status: NEW → ASSIGNED
Comment on attachment 8783074 [details]
Bug 1296760 - Add TestBasePointerCoersion for Maybe. -

https://reviewboard.mozilla.org/r/73042/#review70844

Pretty sure the conversion approach is the desirable one here.
Attachment #8783074 - Flags: review?(jwalden+bmo) → review-
Comment on attachment 8783115 [details] [diff] [review]
Allow Some(nullptr) and Some(Derived*) to convert to Maybe<Base*>

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

Such tests.
Attachment #8783115 - Flags: review?(nfroyd) → review+
Pushed by jwalden@mit.edu:
https://hg.mozilla.org/integration/mozilla-inbound/rev/837df946deca
Allow Some(nullptr) and Some(Derived*) to convert to Maybe<Base*>.  r=froydnj
https://hg.mozilla.org/mozilla-central/rev/837df946deca
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla51
Comment on attachment 8783074 [details]
Bug 1296760 - Add TestBasePointerCoersion for Maybe. -

https://reviewboard.mozilla.org/r/73042/#review72976

I landed the patchwork for this bug over the weekend, and it included tests for derived pointers in base pointers and all that, so I don't think this adds extra value.  But I didn't compare this against what I landed, so if I missed some nuance, feel free to rebase this, point the nuance out to me, and we can reevaluate.
Attachment #8783074 - Flags: review?(jwalden+bmo)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: