Closed
Bug 1461450
Opened 7 years ago
Closed 6 years ago
AutoTArray<T, N> objects do not have move constructors
Categories
(Core :: XPCOM, enhancement)
Core
XPCOM
Tracking
()
RESOLVED
FIXED
mozilla63
Tracking | Status | |
---|---|---|
firefox63 | --- | fixed |
People
(Reporter: nika, Assigned: nika)
References
Details
Attachments
(2 files, 1 obsolete file)
1.17 KB,
patch
|
Details | Diff | Splinter Review | |
1.45 KB,
patch
|
erahm
:
review+
|
Details | Diff | Splinter Review |
They only define copy constructors and generic move constructors. A struct which contains an AutoTArray will always copy that array when being moved.
Bug 1185794 points out that it was ignored due to the issues with differently-sized objects, but that doesn't apply to identically-sized types.
We should support moving AutoTArray for identically-sized buffers.
Assignee | ||
Comment 1•7 years ago
|
||
Attachment #8975648 -
Flags: review?(nfroyd)
Assignee | ||
Updated•7 years ago
|
Assignee: nobody → nika
Comment 2•7 years ago
|
||
Comment on attachment 8975648 [details] [diff] [review]
Support moving AutoTArray objects
Review of attachment 8975648 [details] [diff] [review]:
-----------------------------------------------------------------
Tests for this would be ideal.
Attachment #8975648 -
Flags: review?(nfroyd) → review+
Comment 3•6 years ago
|
||
Nika, do you have time to get this landed? I think all we need is to add a simple test to `test_move_array` [1].
[1] https://searchfox.org/mozilla-central/rev/1193ef6a61cb6e350460eb2e8468184d3cb0321d/xpcom/tests/gtest/TestTArray2.cpp#300
Blocks: 1455891
Flags: needinfo?(nika)
Assignee | ||
Comment 4•6 years ago
|
||
Assignee | ||
Updated•6 years ago
|
Attachment #8975648 -
Attachment is obsolete: true
Assignee | ||
Comment 5•6 years ago
|
||
Attachment #8990477 -
Flags: review?(erahm)
Comment 6•6 years ago
|
||
Comment on attachment 8990477 [details] [diff] [review]
Part 2: Add tests for AutoTArray move constructors
Review of attachment 8990477 [details] [diff] [review]:
-----------------------------------------------------------------
Looks good, thanks for adding the tests.
Attachment #8990477 -
Flags: review?(erahm) → review+
Pushed by nika@thelayzells.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/2a02d86bc113
Part 1: Add move constructors and assignment operators to nsTArray, r=froydnj
https://hg.mozilla.org/integration/mozilla-inbound/rev/6e8148c1ca7f
Part 2: Add tests for AutoTArray move constructors, r=erahm
Comment 8•6 years ago
|
||
Backed out 4 changesets (bug 1461450, bug 1471726) for linting failure xpidl.py
push that caused the backout: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=6e8148c1ca7f2cdf82b4f732aadbf6c8a0d1c212&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-classifiedState=unclassified
failure: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-classifiedState=unclassified&selectedJob=186936069&filter-searchStr=Linting+opt+source-test-mozlint-py-flake8+%28f8%29
log: https://treeherder.mozilla.org/logviewer.html#?job_id=186936069&repo=mozilla-inbound
backout: https://hg.mozilla.org/integration/mozilla-inbound/rev/cddc86531ae9ceed587594be3f9715a43d78efc7
Pushed by nika@thelayzells.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/a5d04cf5d67f
Part 1: Add move constructors and assignment operators to nsTArray, r=froydnj
https://hg.mozilla.org/integration/mozilla-inbound/rev/44414b1cd32e
Part 2: Add tests for AutoTArray move constructors, r=erahm
Comment 10•6 years ago
|
||
Backed out 15 changesets (bug 1475409, bug 1461450, bug 1474369, bug 1471726) for build bustages on xptcstubs_gcc_x86_unix.cpp:55:1.
Backout: https://hg.mozilla.org/integration/mozilla-inbound/rev/113e9ca0b5bccaf3eaee398e789f9b7b8c226009
Push with failures: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=79dbf5b9d8db577bba582a0853eb293d80eed0ba&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&selectedJob=190109107
Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=190109107&repo=mozilla-inbound&lineNumber=12152
Comment 11•6 years ago
|
||
Pushed by nika@thelayzells.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/961379be0f5e
Part 1: Add move constructors and assignment operators to nsTArray, r=froydnj
https://hg.mozilla.org/integration/mozilla-inbound/rev/c721ada8a6d6
Part 2: Add tests for AutoTArray move constructors, r=erahm
Comment 12•6 years ago
|
||
Backed out for causing rooting hazards and browser chrome failures
Push with failures: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&filter-searchStr=Linux%20x64%20pgo%20Mochitests%20with%20e10s%20test-linux64-pgo%2Fopt-mochitest-browser-chrome-e10s-1%20M-e10s(bc1)&fromchange=7ce27aa3ce6887c226baa88223c0bf95bc2c3c28&tochange=e4f654755cc5cb80fb0a5a91707e8a2eff425e3e&selectedJob=190915193
https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&fromchange=7ce27aa3ce6887c226baa88223c0bf95bc2c3c28&tochange=e4f654755cc5cb80fb0a5a91707e8a2eff425e3e&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=success&filter-classifiedState=unclassified&selectedJob=190903190&filter-searchStr=Linux%20x64%20debug%20hazard-linux64-haz%2Fdebug%20(H)
Failure log:
browser chrome failures: https://treeherder.mozilla.org/logviewer.html#?job_id=190915193&repo=mozilla-inbound&lineNumber=3614
rooting hazards: https://treeherder.mozilla.org/logviewer.html#?job_id=190903190&repo=mozilla-inbound&lineNumber=47845
Backout link: https://hg.mozilla.org/integration/mozilla-inbound/rev/e4f654755cc5cb80fb0a5a91707e8a2eff425e3e
Comment 13•6 years ago
|
||
Pushed by nika@thelayzells.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/252e00b20a4a
Part 1: Add move constructors and assignment operators to nsTArray, r=froydnj
https://hg.mozilla.org/integration/mozilla-inbound/rev/2084dd0494b5
Part 2: Add tests for AutoTArray move constructors, r=erahm
Comment 14•6 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/252e00b20a4a
https://hg.mozilla.org/mozilla-central/rev/2084dd0494b5
Status: NEW → RESOLVED
Closed: 6 years ago
status-firefox63:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla63
Assignee | ||
Updated•6 years ago
|
Flags: needinfo?(nika)
You need to log in
before you can comment on or make changes to this bug.
Description
•