Closed Bug 1156802 Opened 7 years ago Closed 7 years ago

Do not allow move constructors to be explicit


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

Not set


(firefox40 affected, firefox43 fixed)

Tracking Status
firefox40 --- affected
firefox43 --- fixed


(Reporter: botond, Assigned: nika)



(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+
Closed: 7 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.