Remove mozilla::Unused and unused_t
Categories
(Core :: MFBT, task, P3)
Tracking
()
People
(Reporter: cpeterson, Unassigned)
References
(Depends on 2 open bugs, Blocks 1 open bug)
Details
(Keywords: good-first-bug)
Attachments
(2 files)
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•4 years ago
|
||
(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 ofMOZ_UNUSED
at this time (suppressing gcc warnings about unusedwrite()
return values that can't be suppressed by casting tovoid
):
I wonder if there is a reason why FdPrintf()
at least doesn't handle EINTR.
Comment 2•4 years ago
|
||
build/clang-plugin/KungFuDeathGripChecker.cpp must also be updated, since its diagnostics suggest the use of mozilla::Unused
.
Comment 3•3 years ago
|
||
- 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.
Updated•2 years ago
|
Updated•1 year ago
|
Comment 4•1 year ago
|
||
Updated•1 year ago
|
Comment 5•1 year ago
|
||
Depends on D173711
Updated•1 year ago
|
Updated•1 year ago
|
Comment 6•1 year ago
•
|
||
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•9 months ago
|
||
This good-first-bug hasn't had any activity for 2 months, it is automatically unassigned.
For more information, please visit BugBot documentation.
Description
•