1684092 - Replace MOZ_MUST_USE with [[nodiscard]] in js/src/

Status: RESOLVED FIXED

(Core :: JavaScript Engine, task, P3)

Description

[Chris Peterson [:cpeterson]]

The MOZ_MUST_USE macro is defined as clang's and gcc's nonstandard __attribute__((warn_unused_result)). Now that we compile as C++17 by default (bug 1560664), we can replace MOZ_MUST_USE with C++17's standard [[nodiscard]] attribute.

The [[nodiscard]] attribute must precede a function declaration's declaration specifiers (like static, extern, inline, or virtual). The __attribute__((warn_unused_result)) attribute does not have this order restriction.

Comment 1

[Chris Peterson [:cpeterson]]

Attachment: Bug 1684092 - Part 1: Replace MOZ_MUST_USE with [[nodiscard]] in js/src/. r?jandem

The MOZ_MUST_USE macro is defined as clang's and gcc's nonstandard attribute((warn_unused_result)). Now that we compile as C++17 by default (bug 1560664), we can replace MOZ_MUST_USE with C++17's standard [[nodiscard]] attribute.

Comment 2

[Chris Peterson [:cpeterson]]

Attachment: Bug 1684092 - Part 2: Move [[nodiscard]] to the head of function declarations in js/src/. r?jandem

The [[nodiscard]] attribute must precede a function declaration's declaration specifiers (like static, extern, inline, or virtual). The attribute((warn_unused_result)) attribute does not have this order restriction.

Depends on D100412

Comment 3

[Chris Peterson [:cpeterson]]

Attachment: Bug 1684092 - Part 3: Remove unnecessary #includes of Attributes.h for MOZ_MUST_USE in js/src/. r?jandem

We don't need to #include Attributes.h now that we're using [[nodiscard]] instead of MOZ_MUST_USE.

Depends on D100413

Comment 4

[Chris Peterson [:cpeterson]]

Attachment: Bug 1684092 - Part 4: Revert V8_WARN_UNUSED_RESULT to upstream definition using __attribute__((warn_unused_result)). r?jorendorff

V8_WARN_UNUSED_RESULT can't use [[nodiscard]] because [[nodiscard]] has more restrictions on function declaration specifiers than attribute((warn_unused_result)). The affected code is third-party code from V8's irregexp, so instead of changing upstream code, revert V8_WARN_UNUSED_RESULT to its upstream definition using attribute((warn_unused_result)).

Depends on D100414

Comment 5

[Chris Peterson [:cpeterson]]

Green Try build:

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

Comment 6

[Chris Peterson [:cpeterson]]

Comment on attachment 9194590 [details]
Bug 1684092 - Part 4: Revert V8_WARN_UNUSED_RESULT to upstream definition using attribute((warn_unused_result)). r?jorendorff

Part 4 was obsoleted by bug 1688350 comment 1

Comment 7

[Chris Peterson [:cpeterson]]

I rebased these s/MOZ_MUST_USE/[[nodiscard]]/ find-and-replace patches. They bit rot quickly because they touch a lot of header files.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/bfaa95b5e04a
Part 1: Replace MOZ_MUST_USE with [[nodiscard]] in js/src/. r=jandem
https://hg.mozilla.org/integration/autoland/rev/331046f56c01
Part 2: Move [[nodiscard]] to the head of function declarations in js/src/. r=jandem
https://hg.mozilla.org/integration/autoland/rev/1f385759e3f5
Part 3: Remove unnecessary #includes of Attributes.h for MOZ_MUST_USE in js/src/. r=jandem

Comment 9

[Noemi Erli[:noemi_erli]]

https://hg.mozilla.org/mozilla-central/rev/bfaa95b5e04a
https://hg.mozilla.org/mozilla-central/rev/331046f56c01
https://hg.mozilla.org/mozilla-central/rev/1f385759e3f5