1994397 - Remove deprecated header Unused.h

Status: RESOLVED FIXED

(Core :: MFBT, task)

Description

[[:sergesanspaille]]

This header is flagged as deprecated by config/check_spidermonkey_style.py and can indeed be repalced by a cast to (void)

Comment 1

[[:sergesanspaille]]

Attachment: Bug 1994397 - Remove obsolete Unused.h header r=emilio!

It is replaced by a standard-conforming cast to (void) which is
understood the same by the compiler.

In some cases this could be replaced by [[maybe_unused]] but for
expression whose value is ignored, this requires storing to a temporary
variable first, so not going that way.

One notable change is that marking an object of type already_AddRefed as
unused used to implicitly leak memory. This is now made explicit.

Comment 2

[Karl Tomlinson (:karlt)]

Attachment: WIP: Bug 1994397 sed changes and clang-format

grep -l '\<mozilla::Unused> <<' **/.(cc|cpp|h)(.) | xargs sed -i -e 's/mozilla::Unused << /(void)/g'
grep -l '\<Unused> <<' **/.(cc|cpp|h)(.) | xargs sed -i -e 's/Unused << /(void)/g'
grep -l mozilla/Unused.h **/*.(cc|cpp|h)(.) | xargs sed -i -e '/^# *include "mozilla/Unused.h"$/ d'
./mach lint --rev @ --linter clang-format --fix

Comment 3

[Karl Tomlinson (:karlt)]

Attachment: WIP: Bug 1994397 other changes in https://phabricator.services.mozilla.com/D268706?id=1139102

Comment 4

[Rob Wu [:robwu]]

(In reply to [:sergesanspaille] from comment #0)

This header is flagged as deprecated by config/check_spidermonkey_style.py and can indeed be repalced by a cast to (void)

Apparently this was added in bug 1713212. And there was already a bug tracking its removal, at bug 1628542, also with a note to do a bit more than just replacing Unused << with (void). I have no particular opinion here, I'm flagging this in case it is relevant.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/b23c15e92720
https://hg.mozilla.org/integration/autoland/rev/5650c180278d
Remove obsolete Unused.h header r=emilio,tschuster,geckoview-reviewers,extension-reviewers,application-update-reviewers,media-playback-reviewers,webrtc-reviewers,cookie-reviewers,places-reviewers,profiler-reviewers,win-reviewers,dom-storage-reviewers,permissions-reviewers,profiles-reviewers,toolkit-telemetry-reviewers,firefox-style-system-reviewers,firefox-svg-reviewers,layout-reviewers,timhuang,gstoll,TravisLong,bytesized,mstange,karlt,robwu,pehrsons,m_kato,jari,jhirsch

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/e1e6bbb1c999
https://hg.mozilla.org/integration/autoland/rev/62dbe7e24315
Revert "Bug 1994397 - Remove obsolete Unused.h header r=emilio,tschuster,geckoview-reviewers,extension-reviewers,application-update-reviewers,media-playback-reviewers,webrtc-reviewers,cookie-reviewers,places-reviewers,profiler-reviewers,win-reviewers,dom-storage-reviewers,permissions-reviewers,profiles-reviewers,toolkit-telemetry-reviewers,firefox-style-system-reviewers,firefox-svg-reviewers,layout-reviewers,timhuang,gstoll,TravisLong,bytesized,mstange,karlt,robwu,pehrsons,m_kato,jari,jhirsch" for causing bustages on Document.cpp

Comment 7

[Narcis Beleuzu [:NarcisB]]

Backed out for causing bustages on Document.cpp

Backout link: https://hg-edge.mozilla.org/integration/autoland/rev/62dbe7e243150b1fee1fc9e9523a10606a093fbb
Log link: https://treeherder.mozilla.org/logviewer?job_id=532261382&repo=autoland&task=BE8P1CKfRt6QGwSoirWJ0A.0&lineNumber=67669

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/fd0ffa193c92
https://hg.mozilla.org/integration/autoland/rev/03ba746b86d0
Remove obsolete Unused.h header r=emilio,tschuster,geckoview-reviewers,extension-reviewers,application-update-reviewers,media-playback-reviewers,webrtc-reviewers,cookie-reviewers,places-reviewers,profiler-reviewers,win-reviewers,dom-storage-reviewers,permissions-reviewers,profiles-reviewers,toolkit-telemetry-reviewers,firefox-style-system-reviewers,firefox-svg-reviewers,layout-reviewers,timhuang,gstoll,TravisLong,bytesized,mstange,karlt,robwu,pehrsons,m_kato,jari,jhirsch

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/aab924e90d34
https://hg.mozilla.org/integration/autoland/rev/e77459c91018
Revert "Bug 1994397 - Remove obsolete Unused.h header r=emilio,tschuster,geckoview-reviewers,extension-reviewers,application-update-reviewers,media-playback-reviewers,webrtc-reviewers,cookie-reviewers,places-reviewers,profiler-reviewers,win-reviewers,dom-storage-reviewers,permissions-reviewers,profiles-reviewers,toolkit-telemetry-reviewers,firefox-style-system-reviewers,firefox-svg-reviewers,layout-reviewers,timhuang,gstoll,TravisLong,bytesized,mstange,karlt,robwu,pehrsons,m_kato,jari,jhirsch" for causing build bustage at nsBrowserApp.cpp

Comment 10

[Cristina Horotan [:chorotan]]

Backed out for causing build bustage at nsBrowserApp.cpp

Backout link

Push with failures

Failure log
Failure log gtest

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/e98adca292d8
https://hg.mozilla.org/integration/autoland/rev/b707187501d1
Remove obsolete Unused.h header r=emilio,tschuster,geckoview-reviewers,extension-reviewers,application-update-reviewers,media-playback-reviewers,webrtc-reviewers,cookie-reviewers,places-reviewers,profiler-reviewers,win-reviewers,dom-storage-reviewers,permissions-reviewers,profiles-reviewers,toolkit-telemetry-reviewers,firefox-style-system-reviewers,firefox-svg-reviewers,layout-reviewers,timhuang,gstoll,TravisLong,bytesized,mstange,karlt,robwu,pehrsons,m_kato,jari,jhirsch

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/8a1b1970b7bd
https://hg.mozilla.org/integration/autoland/rev/692fc033323b
Revert "Bug 1994397 - Remove obsolete Unused.h header r=emilio,tschuster,geckoview-reviewers,extension-reviewers,application-update-reviewers,media-playback-reviewers,webrtc-reviewers,cookie-reviewers,places-reviewers,profiler-reviewers,win-reviewers,dom-storage-reviewers,permissions-reviewers,profiles-reviewers,toolkit-telemetry-reviewers,firefox-style-system-reviewers,firefox-svg-reviewers,layout-reviewers,timhuang,gstoll,TravisLong,bytesized,mstange,karlt,robwu,pehrsons,m_kato,jari,jhirsch" for causing Gtest failures on AlreadyAddRefed.h

Comment 13

[Cristina Horotan [:chorotan]]

Backed out for causing Gtest failures on AlreadyAddRefed.h

Backout link

Push with failures

Failure log

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/7662b3abdcca
https://hg.mozilla.org/integration/autoland/rev/6cc3f34d9633
Remove obsolete Unused.h header r=emilio,tschuster,geckoview-reviewers,extension-reviewers,application-update-reviewers,media-playback-reviewers,webrtc-reviewers,cookie-reviewers,places-reviewers,profiler-reviewers,win-reviewers,dom-storage-reviewers,permissions-reviewers,profiles-reviewers,toolkit-telemetry-reviewers,firefox-style-system-reviewers,firefox-svg-reviewers,layout-reviewers,timhuang,gstoll,TravisLong,bytesized,mstange,karlt,robwu,pehrsons,m_kato,jari,jhirsch

Comment 15

[Rob Wu [:robwu]]

There has been 3 immediate backouts on this bug, of a patch that spans over 1000 files.

Once these commits reach the firefox-main repo (and mozilla-central too?), could we append the commit hashes of the 6 commits (3x land & revert) to the .git-blame-ignore-revs and .hg-annotate-ignore-revs files at the top of the repo to make git blame less painful?

• https://searchfox.org/firefox-main/rev/50a34d25155fd70628ee69c7d68a2509c0e3445d/.hg-annotate-ignore-revs
• https://searchfox.org/firefox-main/rev/50a34d25155fd70628ee69c7d68a2509c0e3445d/.git-blame-ignore-revs

(I don't know what the guidelines are for adding/not adding entries, but I would expect multiple giant commits spanning 1000+ files each to make the cut).

Comment 16

[[:sergesanspaille]]

Will do, letting a few days flow beforehand though.

Comment 17

[amarc]

https://hg.mozilla.org/mozilla-central/rev/6cc3f34d9633

Comment 18

[[:sergesanspaille]]

Attachment: Bug 1994397 - Ignore the three backedout changeset associated to Bug 1994397 r=sylvestre

Those 6 commits each touch ~1000 files, they will clutter git blame
history, better ignore them.

Comment 20

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/5378a419d8c1
https://hg.mozilla.org/integration/autoland/rev/31a0b95ddf9d
Ignore the three backedout changeset associated to Bug 1994397 r=sylvestre

Comment 21

[BugBot [:suhaib / :marco/ :calixte]]

A patch has been attached on this bug, which was already closed. Filing a separate bug will ensure better tracking. If this was not by mistake and further action is needed, please alert the appropriate party. (Or: if the patch doesn't change behavior -- e.g. landing a test case, or fixing a typo -- then feel free to disregard this message)

Comment 22

[Norisz Fay [:noriszfay]]

https://hg.mozilla.org/mozilla-central/rev/31a0b95ddf9d