Closed Bug 1156802 Opened 5 years ago Closed 5 years ago

Do not allow move constructors to be explicit

Categories

(Firefox Build System :: Source Code Analysis, defect)

defect
Not set
normal

Tracking

(firefox40 affected, firefox43 fixed)

RESOLVED FIXED
mozilla43
Tracking Status
firefox40 --- affected
firefox43 --- fixed

People

(Reporter: botond, Assigned: nika)

Details

Attachments

(2 files, 1 obsolete file)

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.)
I agree, we should force them to not be explicit.
I'll do this.
Assignee: nobody → michael
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 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+
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
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla43
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.