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

Status: RESOLVED DUPLICATE of bug 1119228

(Core :: JavaScript Engine, defect)

Description

[Daniel Holbert [:dholbert]]

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.

Comment 1

[Daniel Holbert [:dholbert]]

Attachment: hackaround (evil, horrible, do not land)

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.

Comment 2

[Dave Garrett]

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).

Comment 3

[Daniel Holbert [:dholbert]]

Nice! I like it.

Comment 4

[Jason Orendorff [:jorendorff]]

Attachment: Eliminate compiler warnings around JS_OOM_POSSIBLY_FAIL

Comment 5

[Daniel Holbert [:dholbert]]

ni=jorendorff to get this landed

Comment 6

[Jason Orendorff [:jorendorff]]

Already fixed elsewhere; duping forward.