1628542 - Remove mozilla::Unused and unused_t

Status: NEW

(Core :: MFBT, task, P3)

Description

[Chris Peterson [:cpeterson]]

In bug 1059038 comment 7, Waldo said:

I think people should 1) not ignore return values, at the least to assert some characteristic about them, or failing that, 2) just cast to void.

mozilla::Unused was needed because casting to void doesn't suppress gcc's warn_unused_result warnings. It does suppress gcc's and clang's warnings about C++17 [[nodiscard]]. Once we globally replace MOZ_MUST_USE with [[nodiscard]] (in bug 1571631), we can replace uses of Unused << with (void) casts like:

- Unused << SendShowEvent(aData, aFromUser);
+ (void)SendShowEvent(aData, aFromUser);

However, a safer way to ignore return values is to assert function calls that "can never fail" using MOZ_ALWAYS_TRUE()/etc. MOZ_ALWAYS_TRUE(X) evaluates the expression X in both debug and release builds, so you could write:

- Unused << SendShowEvent(aData, aFromUser);
+ MOZ_ALWAYS_TRUE(SendShowEvent(aData, aFromUser));

TBD: Can we also remove MOZ_UNUSED (bug 1202965)? It was added for use in C code. There are only two uses of MOZ_UNUSED at this time (suppressing gcc warnings about unused write() return values that can't be suppressed by casting to void):

https://searchfox.org/mozilla-central/search?case=true&q=MOZ_UNUSED

We could replace them with something like (in order of increasing pessimism);

• MOZ_ALWAYS_TRUE(write(fd, buf, nbytes) == nbytes));
• MOZ_ALWAYS_TRUE(write(fd, buf, nbytes) >= 0));
• MOZ_ALWAYS_TRUE(write(fd, buf, nbytes) >= -1));

Comment 1

[Karl Tomlinson (:karlt)]

(In reply to Chris Peterson [:cpeterson] from comment #0)

TBD: Can we also remove MOZ_UNUSED (bug 1202965)? It was added for use in C code. There are only two uses of MOZ_UNUSED at this time (suppressing gcc warnings about unused write() return values that can't be suppressed by casting to void):

I wonder if there is a reason why FdPrintf() at least doesn't handle EINTR.

Comment 2

[Simon Giesecke [:sg] [he/him]]

build/clang-plugin/KungFuDeathGripChecker.cpp must also be updated, since its diagnostics suggest the use of mozilla::Unused.

Comment 3

[Masatoshi Kimura [:emk]]

• MOZ_ALWAYS_TRUE(write(fd, buf, nbytes) == nbytes));

This is wrong because write has no guarantee to write all requested bytes at once. We have to repeat ourselves to ensure that all requested bytes are written.

Comment 4

[Andi [:andi]]

Attachment: Bug 1628542 - move away from mozilla::Unused in the detriment of (void).

Comment 5

[Andi [:andi]]

Attachment: Bug 1628542 - remove mozilla::Unused

Depends on D173711

Comment 6

[Kagami Rosylight [:saschanaz] (they/them) (OOO until Dec 11)]

Looking at my patch in bug 1826734, I think any Unused << nsresult cases should be converted to something that logs a warning if NS_FAILED. And also for (void)nsresult.

dom/quota has a great helper function named QM_WARNONLY_TRY, it would be great to use that here.

Comment 7

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

This good-first-bug hasn't had any activity for 2 months, it is automatically unassigned.
For more information, please visit BugBot documentation.

Comment 8

[Ali Yousef]

Hello!
I would like to give it a try

Comment 9

[Ali Yousef]

Can you please assign me this one , I want to give it a try!

Comment 10

[Mayank Bansal]

Usually the bug is assigned once the contributor attaches a patch :)

Comment 11

[Titov Dmitry]

Hi, is there any people, who are working on this? If not, I would try to land my first bug.

Comment 12

[Titov Dmitry]

or actually new people can be assigned to these after January, 7th?