Closed
Bug 824840
Opened 8 years ago
Closed 5 years ago
Easy to forget the argument to MOZ_ASSERT_UNREACHABLE()
Categories
(Core :: MFBT, defect)
Core
MFBT
Tracking
()
RESOLVED
WORKSFORME
People
(Reporter: n.nethercote, Unassigned)
References
(Blocks 1 open bug)
Details
We have 16 places in the code base where MOZ_NOT_REACHED() is missing an argument. clang 3.2 warns about these, and if you build with --enable-warnings-as-errors it causes the build to fail. The simple fix is to add the argument to the 16 occurrences, but I wonder if we should make the zero-argument case legitimate instead? This is an easy mistake to make.
Updated•8 years ago
|
Blocks: buildwarning
Comment 1•8 years ago
|
||
(Technically, MOZ_NOT_REACHED_MARKER() is already a no-argument version of MOZ_NOT_REACHED(), but it doesn't look like it's intended for use outside of other macros' implementations in Assertions.h.)
Comment 2•8 years ago
|
||
> I wonder if we should make the zero-argument case legitimate instead?
+1 MOZ_NOT_REACHED("no") is not useful.
Would this require a multi-step macro declaration like MOZ_ASSERT (http://dxr.mozilla.org/mozilla-central/mfbt/Assertions.h.html#l238)? Or is it as simple as changing MOZ_NOT_REACHED(reason) to MOZ_NOT_REACHED(...) and using __VA_ARGS__? I haven't used Visual C++, but this is stopping me from switching to Clang on linux so I'm willing to fix it.
This was renamed to MOZ_ASSUME_UNREACHABLE() and I can see only one use without a argument, which is in some dummy initializer [1]. [1]: http://mxr.mozilla.org/mozilla-central/source/js/src/jsapi.cpp#1147 OK to close this as INVALID?
Flags: needinfo?(n.nethercote)
![]() |
Reporter | |
Comment 5•7 years ago
|
||
Sounds like the problem still exists, even if the macro's name changed.
Flags: needinfo?(n.nethercote)
Summary: Easy to forget the argument to MOZ_NOT_REACHED() → Easy to forget the argument to MOZ_ASSUME_UNREACHABLE()
Comment 6•7 years ago
|
||
See related bug 990764 for discussion of removing or replacing MOZ_ASSUME_UNREACHABLE.
Comment 7•6 years ago
|
||
Bug 990764 removed MOZ_ASSUME_UNREACHABLE, replacing calls with MOZ_ASSERT_UNREACHABLE and MOZ_CRASH.
Summary: Easy to forget the argument to MOZ_ASSUME_UNREACHABLE() → Easy to forget the argument to MOZ_ASSERT_UNREACHABLE()
Comment 8•5 years ago
|
||
We build with --enable-warnings-as-errors by default, so any missing macro arguments would break the build today.
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → WORKSFORME
You need to log in
before you can comment on or make changes to this bug.
Description
•