Closed Bug 1108155 Opened 5 years ago Closed 5 years ago

js/src/jsgcinlines.h:492:5: error: implicit conversion of nullptr constant to 'bool' [-Werror,-Wnull-conversion]

Categories

(Core :: JavaScript Engine, defect)

x86_64
Linux
defect
Not set

Tracking

()

RESOLVED DUPLICATE of bug 1119228

People

(Reporter: dholbert, Assigned: jorendorff)

References

(Blocks 2 open bugs)

Details

Attachments

(2 files)

Build warning treated as error, using clang 3.6:
{
In file included from $SRCDIR/js/src/jsarray.cpp:39:
In file included from $SRCDIR/js/src/vm/ArgumentsObject-inl.h:14:
In file included from $SRCDIR/js/src/jsscriptinlines.h:19:
In file included from $SRCDIR/js/src/vm/Shape-inl.h:22:
$SRCDIR/js/src/jsgcinlines.h:492:5: error: implicit conversion of nullptr constant to 'bool' [-Werror,-Wnull-conversion]
    JS_OOM_POSSIBLY_FAIL();
    ^~~~~~~~~~~~~~~~~~~~~~
../../dist/include/js/Utility.h:88:20: note: expanded from macro 'JS_OOM_POSSIBLY_FAIL'
            return nullptr; \
            ~~~~~~ ^
}

This happens because we have a function "bool PossiblyFail()", which invokes JS_OOM_POSSIBLY_FAIL, which contains the statement "return nullptr".

The compiler is able to convert nullptr to false, but not without a warning (anymore), which we apparently treat as an error.
Flags: needinfo?(jorendorff)
For reference, here's the hackaround that I used locally to get past this. Basically, I just punted the macro-invocation to a helper-function which returns a void*, and I make that helper-function return (void*)1 as a "truthy" value. And then PossiblyFail() can explicitly convert that helper-function's return-value (nullptr or (void*)1) to a boolean.

This "(void*)1" abuse is horrible & evil (though it works), which is why this is just a hackaround; not a fix. Hopefully jorendorff has a better idea for how to restructure this logic.
Simpler & cleaner solution: Change the macro to take an argument for the value to return on failure. Existing usage would be JS_OOM_POSSIBLY_FAIL(nullptr) and usage here would be JS_OOM_POSSIBLY_FAIL(false).
Nice! I like it.
Assignee: nobody → jorendorff
Status: NEW → ASSIGNED
Flags: needinfo?(jorendorff)
Attachment #8535690 - Flags: review?(jdemooij) → review+
ni=jorendorff to get this landed
Flags: needinfo?(jorendorff)
Already fixed elsewhere; duping forward.
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Flags: needinfo?(jorendorff)
Resolution: --- → DUPLICATE
Duplicate of bug: 1119228
You need to log in before you can comment on or make changes to this bug.