Closed Bug 824840 Opened 7 years ago Closed 4 years ago
Easy to forget the argument to MOZ
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.
(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.)
> 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 . : http://mxr.mozilla.org/mozilla-central/source/js/src/jsapi.cpp#1147 OK to close this as INVALID?
Sounds like the problem still exists, even if the macro's name changed.
Summary: Easy to forget the argument to MOZ_NOT_REACHED() → Easy to forget the argument to MOZ_ASSUME_UNREACHABLE()
See related bug 990764 for discussion of removing or replacing MOZ_ASSUME_UNREACHABLE.
Depends on: 990764
OS: Linux → All
Hardware: x86_64 → All
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()
We build with --enable-warnings-as-errors by default, so any missing macro arguments would break the build today.
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → WORKSFORME
You need to log in before you can comment on or make changes to this bug.