Bug 235336 – Fix -Wreturn-type and -Wunused-parameter warnings, January 2022 edition

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Michael Catanzaro]

Fix -Wreturn-type and -Wunused-parameter warnings, January 2022 edition

[Michael Catanzaro]

Created attachment 449433 [details]
Patch

[Michael Catanzaro]

It seems older GCC doesn't like RELEASE_ASSERT_NOT_REACHED() inside constexpr functions.

[Darin Adler]

Comment on attachment 449433 [details]
Patch

Can’t use RELEASE_ASSERT_NOT_REACHED in constexpr functions. RELEASE_ASSERT_UNDER_CONSTEXPR_CONTEXT does work, although we don’t have a straight "NOT_REACHED" version, so we could do the inelegant RELEASE_ASSERT_UNDER_CONSTEXPR_CONTEXT(false)

[Michael Catanzaro]

Created attachment 449439 [details]
Patch

[Michael Catanzaro]

RELEASE_ASSERT_NOT_REACHED() actually does work fine there with GCC 11.

> RELEASE_ASSERT_UNDER_CONSTEXPR_CONTEXT

Oooh, didn't know about that. My current patch uses IGNORE_RETURN_TYPE_WARNINGS_BEGIN/END, but that sounds nicer.

[Michael Catanzaro]

Created attachment 449451 [details]
Patch

[EWS]

Committed r288200 (246172@main): <https://commits.webkit.org/246172@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 449451 [details].

[Radar WebKit Bug Importer]

<rdar://problem/87772730>

[Mikhail R. Gadelha]

Hi Michael,

EWS MIPS bots use gcc 8.4.0 and this patch breaks the build in debug mode... could you take a look?

[Mikhail R. Gadelha]

The error message:

In file included from WTF/Headers/wtf/StdLibExtras.h:33,
                 from WTF/Headers/wtf/FastMalloc.h:26,
                 from ../../Source/JavaScriptCore/config.h:38,
                 from ../../Source/JavaScriptCore/assembler/MacroAssemblerCodeRef.cpp:26,
                 from JavaScriptCore/DerivedSources/unified-sources/UnifiedSource-cd2e8cfa-2.cpp:1:
../../Source/JavaScriptCore/wasm/WasmCompilationMode.h: In function ‘constexpr bool JSC::Wasm::isOSREntry(JSC::Wasm::CompilationMode)’:
WTF/Headers/wtf/Assertions.h:364:34: error: call to non-‘constexpr’ function ‘void WTFReportAssertionFailure(const char*, int, const char*, const char*)’
         WTFReportAssertionFailure(__FILE__, __LINE__, WTF_PRETTY_FUNCTION, #assertion); \
         ~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
WTF/Headers/wtf/Assertions.h:661:59: note: in expansion of macro ‘ASSERT_UNDER_CONSTEXPR_CONTEXT’
 #define RELEASE_ASSERT_UNDER_CONSTEXPR_CONTEXT(assertion) ASSERT_UNDER_CONSTEXPR_CONTEXT(assertion)
                                                           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../../Source/JavaScriptCore/wasm/WasmCompilationMode.h:53:5: note: in expansion of macro ‘RELEASE_ASSERT_UNDER_CONSTEXPR_CONTEXT’
     RELEASE_ASSERT_UNDER_CONSTEXPR_CONTEXT(false);
     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../../Source/JavaScriptCore/wasm/WasmCompilationMode.h: In function ‘constexpr bool JSC::Wasm::isAnyBBQ(JSC::Wasm::CompilationMode)’:
WTF/Headers/wtf/Assertions.h:364:34: error: call to non-‘constexpr’ function ‘void WTFReportAssertionFailure(const char*, int, const char*, const char*)’
         WTFReportAssertionFailure(__FILE__, __LINE__, WTF_PRETTY_FUNCTION, #assertion); \
         ~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
WTF/Headers/wtf/Assertions.h:661:59: note: in expansion of macro ‘ASSERT_UNDER_CONSTEXPR_CONTEXT’
 #define RELEASE_ASSERT_UNDER_CONSTEXPR_CONTEXT(assertion) ASSERT_UNDER_CONSTEXPR_CONTEXT(assertion)
                                                           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../../Source/JavaScriptCore/wasm/WasmCompilationMode.h:68:5: note: in expansion of macro ‘RELEASE_ASSERT_UNDER_CONSTEXPR_CONTEXT’
     RELEASE_ASSERT_UNDER_CONSTEXPR_CONTEXT(false);
     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../../Source/JavaScriptCore/wasm/WasmCompilationMode.h: In function ‘constexpr bool JSC::Wasm::isAnyOMG(JSC::Wasm::CompilationMode)’:
WTF/Headers/wtf/Assertions.h:364:34: error: call to non-‘constexpr’ function ‘void WTFReportAssertionFailure(const char*, int, const char*, const char*)’
         WTFReportAssertionFailure(__FILE__, __LINE__, WTF_PRETTY_FUNCTION, #assertion); \
         ~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
WTF/Headers/wtf/Assertions.h:661:59: note: in expansion of macro ‘ASSERT_UNDER_CONSTEXPR_CONTEXT’
 #define RELEASE_ASSERT_UNDER_CONSTEXPR_CONTEXT(assertion) ASSERT_UNDER_CONSTEXPR_CONTEXT(assertion)
                                                           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../../Source/JavaScriptCore/wasm/WasmCompilationMode.h:83:5: note: in expansion of macro ‘RELEASE_ASSERT_UNDER_CONSTEXPR_CONTEXT’
     RELEASE_ASSERT_UNDER_CONSTEXPR_CONTEXT(false);
     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

[Michael Catanzaro]

Hi Mikhail, that should have been fixed in bug #235401.

[Michael Catanzaro]

Ah, actually I see that fixed RELEASE_ASSERT_UNDER_CONSTEXPR_CONTEXT(), but your problem is with the normal ASSERT_UNDER_CONSTEXPR_CONTEXT(). The problem is identical, though: the assert just doesn't work at all.

I think we simply need to remove the call to WTFReportAssertionFailure() from ASSERT_UNDER_CONSTEXPR_CONTEXT(), since it's not constexpr. Does it build with that change? If so, please report a bug and I'll see if anybody objects to that....