Closed
Bug 1296760
Opened 8 years ago
Closed 8 years ago
Some(nullptr) should coerce freely into Maybe<T*>
Categories
(Core :: MFBT, defect)
Tracking
()
RESOLVED
FIXED
mozilla51
Tracking | Status | |
---|---|---|
firefox51 | --- | fixed |
People
(Reporter: jgilbert, Assigned: Waldo)
Details
Attachments
(2 files)
58 bytes,
text/x-review-board-request
|
Details | |
5.16 KB,
patch
|
froydnj
:
review+
|
Details | Diff | Splinter Review |
No description provided.
Comment hidden (mozreview-request) |
Assignee | ||
Comment 2•8 years ago
|
||
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 | ||
Updated•8 years ago
|
Assignee: jgilbert → jwalden+bmo
Status: NEW → ASSIGNED
Assignee | ||
Comment 3•8 years ago
|
||
mozreview-review |
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 hidden (mozreview-request) |
Comment 5•8 years ago
|
||
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
Comment 7•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/837df946deca
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
status-firefox51:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla51
Assignee | ||
Comment 8•8 years ago
|
||
mozreview-review |
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.
Description
•