Do not allow move constructors to be explicit

RESOLVED FIXED in Firefox 43

Status

defect
RESOLVED FIXED
4 years ago
a year ago

People

(Reporter: botond, Assigned: Nika)

Tracking

Trunk
mozilla43

Firefox Tracking Flags

(firefox40 affected, firefox43 fixed)

Details

Attachments

(2 attachments, 1 obsolete attachment)

Reporter

Description

4 years ago
Is there a use case for making a move constructor explicit?

If not, then - given that a move constructor being explicit has bitten us before (see bug 1156538) - perhaps we should enforce, via static analysis, that they *not* be explicit?

(Suggested by :froydnj in bug 1156538 comment 5.)

Comment 1

4 years ago
I agree, we should force them to not be explicit.
I'll do this.
Assignee: nobody → michael

Comment 6

4 years ago
Comment on attachment 8656750 [details] [diff] [review]
Part 1: Add an analysis which prohibits explicit move constructors

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

::: build/clang-plugin/clang-plugin.cpp
@@ +793,5 @@
> +
> +// These two matchers are avaliable on more recent versions of clang, but are
> +// not avaliable on all versions which we support, or on our testing
> +// infrastructure. For that reason we prefix them with `moz`, so that the
> +// name does not conflict if we are using an up-to-date build of clang

Instead of doing this, let's add an isExplicitMoveCtor() matcher that combines both of these.
Attachment #8656750 - Flags: review?(ehsan) → review-

Comment 7

4 years ago
Comment on attachment 8656752 [details] [diff] [review]
Part 2: Remove all explicit move constructors

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

r=me on the dom parts, over to Nathan for XPCOM and MFBT.
Attachment #8656752 - Flags: review?(nfroyd)
Attachment #8656752 - Flags: review?(ehsan)
Attachment #8656752 - Flags: review+

Updated

4 years ago
Attachment #8657123 - Flags: review?(ehsan) → review+
Attachment #8656752 - Flags: review?(nfroyd) → review+
https://hg.mozilla.org/mozilla-central/rev/63f3a49b15cb
https://hg.mozilla.org/mozilla-central/rev/c1cdde5ccb2c
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla43

Updated

a year ago
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.