Closed Bug 1156538 Opened 6 years ago Closed 6 years ago

nsTArray move constructor should not be explicit

Categories

(Core :: XPCOM, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla40
Tracking Status
firefox40 --- fixed

People

(Reporter: botond, Assigned: botond)

References

Details

Attachments

(1 file, 1 obsolete file)

The nsTArray move constructor is explicit.

This prevents returning a local variable of type nsTArray by value (using the move constructor to do so). For example:

  #include "nsTArray.h"

  nsTArray<int> foo() {
    nsTArray<int> result;
    return result;
  }

produces:

  error: no matching constructor for initialization of 'nsTArray<int>'
    return result;
           ^~~~~~

(The patch in bug 982212 comment 20 did this for the 'nsTArray(nsTArray_Impl<E, Allocator>&&)' constructor. I believe it should have done so for the regular move constructor as well.)
I should mention that the above code compiles with gcc, which appears to use the 'nsTArray(nsTArray_Impl<E, Allocator>&&)' constructor. My guess is that's a gcc bug.
(The compiler with which it fails to compile is clang.)
Attached file MozReview Request: bz://1156538/botond (obsolete) —
/r/7345 - Bug 1156538 - Make nsTArray's move constructor implicit. r=froydnj

Pull down this commit:

hg pull -r 04d7e84b5df0689dd410350f08e01843cd6a97ed https://reviewboard-hg.mozilla.org/gecko/
Attachment #8595032 - Flags: review?(nfroyd)
Attachment #8595032 - Flags: review?(nfroyd) → review+
Comment on attachment 8595032 [details]
MozReview Request: bz://1156538/botond

https://reviewboard.mozilla.org/r/7343/#review6115

Ship It!
Maybe we should consider relaxing the static analysis for move constructors, or not permitting |explicit| on move constructors, or something?  (Admittedly, this does open some holes...)  Could you file a followup bug for that, please?
(In reply to Nathan Froyd [:froydnj] [:nfroyd] from comment #5)
> Maybe we should consider relaxing the static analysis for move constructors,
> or not permitting |explicit| on move constructors, or something? 
> (Admittedly, this does open some holes...)  Could you file a followup bug
> for that, please?

Filed bug 1156802.
https://hg.mozilla.org/mozilla-central/rev/d1fc7cbc7ff6
Assignee: nobody → botond
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla40
(In reply to Botond Ballo [:botond] from comment #1)
> I should mention that the above code compiles with gcc, which appears to use
> the 'nsTArray(nsTArray_Impl<E, Allocator>&&)' constructor. My guess is
> that's a gcc bug.

I filed https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65869 for this.
Attachment #8595032 - Attachment is obsolete: true
Attachment #8620098 - Flags: review+
You need to log in before you can comment on or make changes to this bug.