1156802 - Do not allow move constructors to be explicit

Status: RESOLVED FIXED

(Developer Infrastructure :: Source Code Analysis, defect)

Description

[Botond Ballo [:botond]]

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

[(no longer active)]

I agree, we should force them to not be explicit.

Comment 2

[Nika Layzell [:nika] (ni? for response)]

I'll do this.

Comment 3

[Nika Layzell [:nika] (ni? for response)]

Attachment: Part 1: Add an analysis which prohibits explicit move constructors

Comment 4

[Nika Layzell [:nika] (ni? for response)]

Attachment: Part 2: Remove all explicit move constructors

Comment 5

[Nika Layzell [:nika] (ni? for response)]

try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=ab2a3e472439

Comment 6

[(no longer active)]

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.

Comment 7

[(no longer active)]

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.

Comment 8

[Nika Layzell [:nika] (ni? for response)]

Attachment: Part 1: Add an analysis which prohibits explicit move constructors

Comment 9

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/63f3a49b15cb
https://hg.mozilla.org/integration/mozilla-inbound/rev/c1cdde5ccb2c

Comment 10

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/63f3a49b15cb
https://hg.mozilla.org/mozilla-central/rev/c1cdde5ccb2c