| Summary: | Fix -Wreturn-type and -Wunused-parameter warnings, January 2022 edition | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Michael Catanzaro <mcatanzaro> | ||||||||
| Component: | WebCore Misc. | Assignee: | Michael Catanzaro <mcatanzaro> | ||||||||
| Status: | RESOLVED FIXED | ||||||||||
| Severity: | Normal | CC: | aperez, bugs-noreply, darin, dino, esprehn+autocc, ews-watchlist, glenn, gyuyoung.kim, keith_miller, kondapallykalyan, macpherson, mark.lam, mcatanzaro, menard, mikhail, msaboff, saam, tzagallo, webkit-bug-importer | ||||||||
| Priority: | P2 | Keywords: | InRadar | ||||||||
| Version: | WebKit Nightly Build | ||||||||||
| Hardware: | PC | ||||||||||
| OS: | Linux | ||||||||||
| See Also: | https://bugs.webkit.org/show_bug.cgi?id=235401 | ||||||||||
| Attachments: |
|
||||||||||
|
Description
Michael Catanzaro
2022-01-18 15:25:00 PST
Created attachment 449433 [details]
Patch
It seems older GCC doesn't like RELEASE_ASSERT_NOT_REACHED() inside constexpr functions. 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)
Created attachment 449439 [details]
Patch
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.
Created attachment 449451 [details]
Patch
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]. Hi Michael, EWS MIPS bots use gcc 8.4.0 and this patch breaks the build in debug mode... could you take a look? 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);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Hi Mikhail, that should have been fixed in bug #235401. 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.... |