Bug 211701 – -Wtype-limits warning spam from CCallHelpers.h

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Michael Catanzaro]

The stable release of GCC 10 has started warning about some preexisting code in CCallHelpers.h. This one is really long, but I'm going to copy the whole thing because the template instantiation is important here. It's four separate warnings; I've added a newline between each one for readability:

[821/2747] Building CXX object Source/JavaScriptCore/CMak...ScriptCore/unified-sources/UnifiedSource-f0a787a9-1.cpp.o
In file included from /home/mcatanzaro/Projects/WebKit/Source/JavaScriptCore/bytecode/AccessCase.cpp:31,
                 from DerivedSources/JavaScriptCore/unified-sources/UnifiedSource-f0a787a9-1.cpp:1:
/home/mcatanzaro/Projects/WebKit/Source/JavaScriptCore/jit/CCallHelpers.h: In instantiation of ‘std::array<RegType, NumberOfRegisters> JSC::CCallHelpers::clampArrayToSize(std::array<RegType, decltype (JSC::toInfoFromReg((RegType)(-1)))::numberOfRegisters>) [with unsigned int TargetSize = 0; RegType = JSC::X86Registers::XMMRegisterID; decltype (JSC::toInfoFromReg((RegType)(-1))) = JSC::FPRInfo]’:
/home/mcatanzaro/Projects/WebKit/Source/JavaScriptCore/jit/CCallHelpers.h:645:85:   required from ‘void JSC::CCallHelpers::setupArgumentsImpl(JSC::CCallHelpers::ArgCollection<numGPRArgs, numGPRSources, numFPRArgs, numFPRSources, numCrossSources, extraGPRArgs, extraPoke>) [with OperationType = char*(JSC::VM*, JSC::JSObject*); unsigned int numGPRArgs = 2; unsigned int numGPRSources = 1; unsigned int numFPRArgs = 0; unsigned int numFPRSources = 0; unsigned int numCrossSources = 0; unsigned int extraGPRArgs = 0; unsigned int extraPoke = 0]’
/home/mcatanzaro/Projects/WebKit/Source/JavaScriptCore/jit/CCallHelpers.h:381:46:   required from ‘void JSC::CCallHelpers::marshallArgumentRegister(JSC::CCallHelpers::ArgCollection<numGPRArgs, numGPRSources, numFPRArgs, numFPRSources, numCrossSources, extraGPRArgs, extraPoke>, RegType, Args ...) [with OperationType = char*(JSC::VM*, JSC::JSObject*); unsigned int numGPRArgs = 1; unsigned int numGPRSources = 0; unsigned int numFPRArgs = 0; unsigned int numFPRSources = 0; unsigned int numCrossSources = 0; unsigned int extraGPRArgs = 0; unsigned int extraPoke = 0; RegType = JSC::X86Registers::RegisterID; Args = {}]’
/home/mcatanzaro/Projects/WebKit/Source/JavaScriptCore/jit/CCallHelpers.h:399:48:   required from ‘void JSC::CCallHelpers::setupArgumentsImpl(JSC::CCallHelpers::ArgCollection<numGPRArgs, numGPRSources, numFPRArgs, numFPRSources, numCrossSources, extraGPRArgs, extraPoke>, JSC::GPRReg, Args ...) [with OperationType = char*(JSC::VM*, JSC::JSObject*); unsigned int numGPRArgs = 1; unsigned int numGPRSources = 0; unsigned int numFPRArgs = 0; unsigned int numFPRSources = 0; unsigned int numCrossSources = 0; unsigned int extraGPRArgs = 0; unsigned int extraPoke = 0; Args = {}; JSC::GPRReg = JSC::X86Registers::RegisterID]’
/home/mcatanzaro/Projects/WebKit/Source/JavaScriptCore/jit/CCallHelpers.h:579:46:   required from ‘std::enable_if_t<(std::is_base_of<JSC::AbstractMacroAssembler<JSC::X86Assembler>::TrustedImm, Arg>::value || std::is_convertible<Arg, JSC::AbstractMacroAssembler<JSC::X86Assembler>::TrustedImm>::value)> JSC::CCallHelpers::setupArgumentsImpl(JSC::CCallHelpers::ArgCollection<numGPRArgs, numGPRSources, numFPRArgs, numFPRSources, numCrossSources, extraGPRArgs, extraPoke>, Arg, Args ...) [with OperationType = char*(JSC::VM*, JSC::JSObject*); unsigned int numGPRArgs = 0; unsigned int numGPRSources = 0; unsigned int numFPRArgs = 0; unsigned int numFPRSources = 0; unsigned int numCrossSources = 0; unsigned int extraGPRArgs = 0; unsigned int extraPoke = 0; Arg = JSC::AbstractMacroAssembler<JSC::X86Assembler>::TrustedImmPtr; Args = {JSC::X86Registers::RegisterID}; std::enable_if_t<(std::is_base_of<JSC::AbstractMacroAssembler<JSC::X86Assembler>::TrustedImm, Arg>::value || std::is_convertible<Arg, JSC::AbstractMacroAssembler<JSC::X86Assembler>::TrustedImm>::value)> = void]’
/home/mcatanzaro/Projects/WebKit/Source/JavaScriptCore/jit/CCallHelpers.h:669:63:   required from ‘std::enable_if_t<(! std::is_same<typename WTF::FunctionTraits<T>::ArgumentType<0>, JSC::CallFrame*>::value)> JSC::CCallHelpers::setupArguments(Args ...) [with OperationType = char*(JSC::VM*, JSC::JSObject*); Args = {JSC::AbstractMacroAssembler<JSC::X86Assembler>::TrustedImmPtr, JSC::X86Registers::RegisterID}; std::enable_if_t<(! std::is_same<typename WTF::FunctionTraits<T>::ArgumentType<0>, JSC::CallFrame*>::value)> = void; typename WTF::FunctionTraits<T>::ArgumentType<0> = JSC::VM*]’
/home/mcatanzaro/Projects/WebKit/Source/JavaScriptCore/bytecode/AccessCase.cpp:1851:161:   required from here
/home/mcatanzaro/Projects/WebKit/Source/JavaScriptCore/jit/CCallHelpers.h:316:32: warning: comparison of unsigned expression in ‘< 0’ is always false [-Wtype-limits]
  316 |         for (unsigned i = 0; i < TargetSize; i++) {
      |                              ~~^~~~~~~~~~~~

/home/mcatanzaro/Projects/WebKit/Source/JavaScriptCore/jit/CCallHelpers.h: In instantiation of ‘void JSC::CCallHelpers::setupStubArgs(std::array<RegType, NumberOfRegisters>, std::array<RegType, NumberOfRegisters>) [with unsigned int NumberOfRegisters = 0; RegType = JSC::X86Registers::XMMRegisterID]’:
/home/mcatanzaro/Projects/WebKit/Source/JavaScriptCore/jit/CCallHelpers.h:645:45:   required from ‘void JSC::CCallHelpers::setupArgumentsImpl(JSC::CCallHelpers::ArgCollection<numGPRArgs, numGPRSources, numFPRArgs, numFPRSources, numCrossSources, extraGPRArgs, extraPoke>) [with OperationType = char*(JSC::VM*, JSC::JSObject*); unsigned int numGPRArgs = 2; unsigned int numGPRSources = 1; unsigned int numFPRArgs = 0; unsigned int numFPRSources = 0; unsigned int numCrossSources = 0; unsigned int extraGPRArgs = 0; unsigned int extraPoke = 0]’
/home/mcatanzaro/Projects/WebKit/Source/JavaScriptCore/jit/CCallHelpers.h:381:46:   required from ‘void JSC::CCallHelpers::marshallArgumentRegister(JSC::CCallHelpers::ArgCollection<numGPRArgs, numGPRSources, numFPRArgs, numFPRSources, numCrossSources, extraGPRArgs, extraPoke>, RegType, Args ...) [with OperationType = char*(JSC::VM*, JSC::JSObject*); unsigned int numGPRArgs = 1; unsigned int numGPRSources = 0; unsigned int numFPRArgs = 0; unsigned int numFPRSources = 0; unsigned int numCrossSources = 0; unsigned int extraGPRArgs = 0; unsigned int extraPoke = 0; RegType = JSC::X86Registers::RegisterID; Args = {}]’
/home/mcatanzaro/Projects/WebKit/Source/JavaScriptCore/jit/CCallHelpers.h:399:48:   required from ‘void JSC::CCallHelpers::setupArgumentsImpl(JSC::CCallHelpers::ArgCollection<numGPRArgs, numGPRSources, numFPRArgs, numFPRSources, numCrossSources, extraGPRArgs, extraPoke>, JSC::GPRReg, Args ...) [with OperationType = char*(JSC::VM*, JSC::JSObject*); unsigned int numGPRArgs = 1; unsigned int numGPRSources = 0; unsigned int numFPRArgs = 0; unsigned int numFPRSources = 0; unsigned int numCrossSources = 0; unsigned int extraGPRArgs = 0; unsigned int extraPoke = 0; Args = {}; JSC::GPRReg = JSC::X86Registers::RegisterID]’
/home/mcatanzaro/Projects/WebKit/Source/JavaScriptCore/jit/CCallHelpers.h:579:46:   required from ‘std::enable_if_t<(std::is_base_of<JSC::AbstractMacroAssembler<JSC::X86Assembler>::TrustedImm, Arg>::value || std::is_convertible<Arg, JSC::AbstractMacroAssembler<JSC::X86Assembler>::TrustedImm>::value)> JSC::CCallHelpers::setupArgumentsImpl(JSC::CCallHelpers::ArgCollection<numGPRArgs, numGPRSources, numFPRArgs, numFPRSources, numCrossSources, extraGPRArgs, extraPoke>, Arg, Args ...) [with OperationType = char*(JSC::VM*, JSC::JSObject*); unsigned int numGPRArgs = 0; unsigned int numGPRSources = 0; unsigned int numFPRArgs = 0; unsigned int numFPRSources = 0; unsigned int numCrossSources = 0; unsigned int extraGPRArgs = 0; unsigned int extraPoke = 0; Arg = JSC::AbstractMacroAssembler<JSC::X86Assembler>::TrustedImmPtr; Args = {JSC::X86Registers::RegisterID}; std::enable_if_t<(std::is_base_of<JSC::AbstractMacroAssembler<JSC::X86Assembler>::TrustedImm, Arg>::value || std::is_convertible<Arg, JSC::AbstractMacroAssembler<JSC::X86Assembler>::TrustedImm>::value)> = void]’
/home/mcatanzaro/Projects/WebKit/Source/JavaScriptCore/jit/CCallHelpers.h:669:63:   required from ‘std::enable_if_t<(! std::is_same<typename WTF::FunctionTraits<T>::ArgumentType<0>, JSC::CallFrame*>::value)> JSC::CCallHelpers::setupArguments(Args ...) [with OperationType = char*(JSC::VM*, JSC::JSObject*); Args = {JSC::AbstractMacroAssembler<JSC::X86Assembler>::TrustedImmPtr, JSC::X86Registers::RegisterID}; std::enable_if_t<(! std::is_same<typename WTF::FunctionTraits<T>::ArgumentType<0>, JSC::CallFrame*>::value)> = void; typename WTF::FunctionTraits<T>::ArgumentType<0> = JSC::VM*]’
/home/mcatanzaro/Projects/WebKit/Source/JavaScriptCore/bytecode/AccessCase.cpp:1851:161:   required from here
/home/mcatanzaro/Projects/WebKit/Source/JavaScriptCore/jit/CCallHelpers.h:102:32: warning: comparison of unsigned expression in ‘< 0’ is always false [-Wtype-limits]
  102 |         for (unsigned i = 0; i < NumberOfRegisters; ++i) {
      |                              ~~^~~~~~~~~~~~~~~~~~~

/home/mcatanzaro/Projects/WebKit/Source/JavaScriptCore/jit/CCallHelpers.h: In instantiation of ‘std::array<RegType, NumberOfRegisters> JSC::CCallHelpers::clampArrayToSize(std::array<RegType, decltype (JSC::toInfoFromReg((RegType)(-1)))::numberOfRegisters>) [with unsigned int TargetSize = 0; RegType = JSC::X86Registers::RegisterID; decltype (JSC::toInfoFromReg((RegType)(-1))) = JSC::GPRInfo]’:
/home/mcatanzaro/Projects/WebKit/Source/JavaScriptCore/jit/CCallHelpers.h:639:85:   required from ‘void JSC::CCallHelpers::setupArgumentsImpl(JSC::CCallHelpers::ArgCollection<numGPRArgs, numGPRSources, numFPRArgs, numFPRSources, numCrossSources, extraGPRArgs, extraPoke>) [with OperationType = char*(JSC::VM*, JSC::JSObject*); unsigned int numGPRArgs = 2; unsigned int numGPRSources = 0; unsigned int numFPRArgs = 0; unsigned int numFPRSources = 0; unsigned int numCrossSources = 0; unsigned int extraGPRArgs = 0; unsigned int extraPoke = 0]’
/home/mcatanzaro/Projects/WebKit/Source/JavaScriptCore/jit/CCallHelpers.h:386:42:   required from ‘void JSC::CCallHelpers::marshallArgumentRegister(JSC::CCallHelpers::ArgCollection<numGPRArgs, numGPRSources, numFPRArgs, numFPRSources, numCrossSources, extraGPRArgs, extraPoke>, RegType, Args ...) [with OperationType = char*(JSC::VM*, JSC::JSObject*); unsigned int numGPRArgs = 1; unsigned int numGPRSources = 0; unsigned int numFPRArgs = 0; unsigned int numFPRSources = 0; unsigned int numCrossSources = 0; unsigned int extraGPRArgs = 0; unsigned int extraPoke = 0; RegType = JSC::X86Registers::RegisterID; Args = {}]’
/home/mcatanzaro/Projects/WebKit/Source/JavaScriptCore/jit/CCallHelpers.h:399:48:   required from ‘void JSC::CCallHelpers::setupArgumentsImpl(JSC::CCallHelpers::ArgCollection<numGPRArgs, numGPRSources, numFPRArgs, numFPRSources, numCrossSources, extraGPRArgs, extraPoke>, JSC::GPRReg, Args ...) [with OperationType = char*(JSC::VM*, JSC::JSObject*); unsigned int numGPRArgs = 1; unsigned int numGPRSources = 0; unsigned int numFPRArgs = 0; unsigned int numFPRSources = 0; unsigned int numCrossSources = 0; unsigned int extraGPRArgs = 0; unsigned int extraPoke = 0; Args = {}; JSC::GPRReg = JSC::X86Registers::RegisterID]’
/home/mcatanzaro/Projects/WebKit/Source/JavaScriptCore/jit/CCallHelpers.h:579:46:   required from ‘std::enable_if_t<(std::is_base_of<JSC::AbstractMacroAssembler<JSC::X86Assembler>::TrustedImm, Arg>::value || std::is_convertible<Arg, JSC::AbstractMacroAssembler<JSC::X86Assembler>::TrustedImm>::value)> JSC::CCallHelpers::setupArgumentsImpl(JSC::CCallHelpers::ArgCollection<numGPRArgs, numGPRSources, numFPRArgs, numFPRSources, numCrossSources, extraGPRArgs, extraPoke>, Arg, Args ...) [with OperationType = char*(JSC::VM*, JSC::JSObject*); unsigned int numGPRArgs = 0; unsigned int numGPRSources = 0; unsigned int numFPRArgs = 0; unsigned int numFPRSources = 0; unsigned int numCrossSources = 0; unsigned int extraGPRArgs = 0; unsigned int extraPoke = 0; Arg = JSC::AbstractMacroAssembler<JSC::X86Assembler>::TrustedImmPtr; Args = {JSC::X86Registers::RegisterID}; std::enable_if_t<(std::is_base_of<JSC::AbstractMacroAssembler<JSC::X86Assembler>::TrustedImm, Arg>::value || std::is_convertible<Arg, JSC::AbstractMacroAssembler<JSC::X86Assembler>::TrustedImm>::value)> = void]’
/home/mcatanzaro/Projects/WebKit/Source/JavaScriptCore/jit/CCallHelpers.h:669:63:   required from ‘std::enable_if_t<(! std::is_same<typename WTF::FunctionTraits<T>::ArgumentType<0>, JSC::CallFrame*>::value)> JSC::CCallHelpers::setupArguments(Args ...) [with OperationType = char*(JSC::VM*, JSC::JSObject*); Args = {JSC::AbstractMacroAssembler<JSC::X86Assembler>::TrustedImmPtr, JSC::X86Registers::RegisterID}; std::enable_if_t<(! std::is_same<typename WTF::FunctionTraits<T>::ArgumentType<0>, JSC::CallFrame*>::value)> = void; typename WTF::FunctionTraits<T>::ArgumentType<0> = JSC::VM*]’
/home/mcatanzaro/Projects/WebKit/Source/JavaScriptCore/bytecode/AccessCase.cpp:1851:161:   required from here
/home/mcatanzaro/Projects/WebKit/Source/JavaScriptCore/jit/CCallHelpers.h:316:32: warning: comparison of unsigned expression in ‘< 0’ is always false [-Wtype-limits]
  316 |         for (unsigned i = 0; i < TargetSize; i++) {
      |                              ~~^~~~~~~~~~~~

/home/mcatanzaro/Projects/WebKit/Source/JavaScriptCore/jit/CCallHelpers.h: In instantiation of ‘void JSC::CCallHelpers::setupStubArgs(std::array<RegType, NumberOfRegisters>, std::array<RegType, NumberOfRegisters>) [with unsigned int NumberOfRegisters = 0; RegType = JSC::X86Registers::RegisterID]’:
/home/mcatanzaro/Projects/WebKit/Source/JavaScriptCore/jit/CCallHelpers.h:639:45:   required from ‘void JSC::CCallHelpers::setupArgumentsImpl(JSC::CCallHelpers::ArgCollection<numGPRArgs, numGPRSources, numFPRArgs, numFPRSources, numCrossSources, extraGPRArgs, extraPoke>) [with OperationType = char*(JSC::VM*, JSC::JSObject*); unsigned int numGPRArgs = 2; unsigned int numGPRSources = 0; unsigned int numFPRArgs = 0; unsigned int numFPRSources = 0; unsigned int numCrossSources = 0; unsigned int extraGPRArgs = 0; unsigned int extraPoke = 0]’
/home/mcatanzaro/Projects/WebKit/Source/JavaScriptCore/jit/CCallHelpers.h:386:42:   required from ‘void JSC::CCallHelpers::marshallArgumentRegister(JSC::CCallHelpers::ArgCollection<numGPRArgs, numGPRSources, numFPRArgs, numFPRSources, numCrossSources, extraGPRArgs, extraPoke>, RegType, Args ...) [with OperationType = char*(JSC::VM*, JSC::JSObject*); unsigned int numGPRArgs = 1; unsigned int numGPRSources = 0; unsigned int numFPRArgs = 0; unsigned int numFPRSources = 0; unsigned int numCrossSources = 0; unsigned int extraGPRArgs = 0; unsigned int extraPoke = 0; RegType = JSC::X86Registers::RegisterID; Args = {}]’
/home/mcatanzaro/Projects/WebKit/Source/JavaScriptCore/jit/CCallHelpers.h:399:48:   required from ‘void JSC::CCallHelpers::setupArgumentsImpl(JSC::CCallHelpers::ArgCollection<numGPRArgs, numGPRSources, numFPRArgs, numFPRSources, numCrossSources, extraGPRArgs, extraPoke>, JSC::GPRReg, Args ...) [with OperationType = char*(JSC::VM*, JSC::JSObject*); unsigned int numGPRArgs = 1; unsigned int numGPRSources = 0; unsigned int numFPRArgs = 0; unsigned int numFPRSources = 0; unsigned int numCrossSources = 0; unsigned int extraGPRArgs = 0; unsigned int extraPoke = 0; Args = {}; JSC::GPRReg = JSC::X86Registers::RegisterID]’
/home/mcatanzaro/Projects/WebKit/Source/JavaScriptCore/jit/CCallHelpers.h:579:46:   required from ‘std::enable_if_t<(std::is_base_of<JSC::AbstractMacroAssembler<JSC::X86Assembler>::TrustedImm, Arg>::value || std::is_convertible<Arg, JSC::AbstractMacroAssembler<JSC::X86Assembler>::TrustedImm>::value)> JSC::CCallHelpers::setupArgumentsImpl(JSC::CCallHelpers::ArgCollection<numGPRArgs, numGPRSources, numFPRArgs, numFPRSources, numCrossSources, extraGPRArgs, extraPoke>, Arg, Args ...) [with OperationType = char*(JSC::VM*, JSC::JSObject*); unsigned int numGPRArgs = 0; unsigned int numGPRSources = 0; unsigned int numFPRArgs = 0; unsigned int numFPRSources = 0; unsigned int numCrossSources = 0; unsigned int extraGPRArgs = 0; unsigned int extraPoke = 0; Arg = JSC::AbstractMacroAssembler<JSC::X86Assembler>::TrustedImmPtr; Args = {JSC::X86Registers::RegisterID}; std::enable_if_t<(std::is_base_of<JSC::AbstractMacroAssembler<JSC::X86Assembler>::TrustedImm, Arg>::value || std::is_convertible<Arg, JSC::AbstractMacroAssembler<JSC::X86Assembler>::TrustedImm>::value)> = void]’
/home/mcatanzaro/Projects/WebKit/Source/JavaScriptCore/jit/CCallHelpers.h:669:63:   required from ‘std::enable_if_t<(! std::is_same<typename WTF::FunctionTraits<T>::ArgumentType<0>, JSC::CallFrame*>::value)> JSC::CCallHelpers::setupArguments(Args ...) [with OperationType = char*(JSC::VM*, JSC::JSObject*); Args = {JSC::AbstractMacroAssembler<JSC::X86Assembler>::TrustedImmPtr, JSC::X86Registers::RegisterID}; std::enable_if_t<(! std::is_same<typename WTF::FunctionTraits<T>::ArgumentType<0>, JSC::CallFrame*>::value)> = void; typename WTF::FunctionTraits<T>::ArgumentType<0> = JSC::VM*]’
/home/mcatanzaro/Projects/WebKit/Source/JavaScriptCore/bytecode/AccessCase.cpp:1851:161:   required from here
/home/mcatanzaro/Projects/WebKit/Source/JavaScriptCore/jit/CCallHelpers.h:102:32: warning: comparison of unsigned expression in ‘< 0’ is always false [-Wtype-limits]
  102 |         for (unsigned i = 0; i < NumberOfRegisters; ++i) {
      |                              ~~^~~~~~~~~~~~~~~~~~~

So looking at the code, one would think "well, the template should never be instantiated with TargetSize == or NumberOfRegisters == 0, and therefore this must be a false positive." But in fact, the templates really are being instantiated with these values, according to the warning:

/home/mcatanzaro/Projects/WebKit/Source/JavaScriptCore/jit/CCallHelpers.h: In instantiation of ‘void JSC::CCallHelpers::setupStubArgs(std::array<RegType, NumberOfRegisters>, std::array<RegType, NumberOfRegisters>) [with unsigned int NumberOfRegisters = 0; RegType = JSC::X86Registers::XMMRegisterID]’:

Seems that's happening here:

    template<typename OperationType, typename... Args>
    ALWAYS_INLINE std::enable_if_t<std::is_same<FIRST_ARGUMENT_TYPE, CallFrame*>::value> setupArguments(Args... args)
    {
#if USE(JSVALUE64)
        // This only really works for 64-bit since jsvalue regs mess things up for 32-bit...
        static_assert(FunctionTraits<OperationType>::cCallArity() == sizeof...(Args) + 1, "Basic sanity check");
#endif
        setupArgumentsImpl<OperationType, 0, 0, 0, 0, 0, 0, 0>(ArgCollection<0, 0, 0, 0, 0, 0, 0>(), GPRInfo::callFrameRegister, args...);
    }

    template<typename OperationType, typename... Args>
    ALWAYS_INLINE std::enable_if_t<!std::is_same<FIRST_ARGUMENT_TYPE, CallFrame*>::value> setupArguments(Args... args)
    {
#if USE(JSVALUE64)
        // This only really works for 64-bit since jsvalue regs mess things up for 32-bit...
        static_assert(FunctionTraits<OperationType>::cCallArity() == sizeof...(Args), "Basic sanity check");
#endif
        setupArgumentsImpl<OperationType, 0, 0, 0, 0, 0, 0, 0>(ArgCollection<0, 0, 0, 0, 0, 0, 0>(), args...);
    }

I tried this naive patch to block that case from being reached at runtime, but it's not enough to avoid the warnings:

diff --git a/Source/JavaScriptCore/jit/CCallHelpers.h b/Source/JavaScriptCore/jit/CCallHelpers.h
index 35d509bc3f54..2c3425edd9f8 100644
--- a/Source/JavaScriptCore/jit/CCallHelpers.h
+++ b/Source/JavaScriptCore/jit/CCallHelpers.h
@@ -99,9 +99,11 @@ private:
         typedef std::pair<RegType, RegType> RegPair;
         Vector<RegPair, NumberOfRegisters> pairs;
 
-        for (unsigned i = 0; i < NumberOfRegisters; ++i) {
-            if (sources[i] != destinations[i])
-                pairs.append(std::make_pair(sources[i], destinations[i]));
+        if (NumberOfRegisters > 0) {
+            for (unsigned i = 0; i < NumberOfRegisters; ++i) {
+                if (sources[i] != destinations[i])
+                    pairs.append(std::make_pair(sources[i], destinations[i]));
+            }
         }
 
 #if ASSERT_ENABLED
@@ -313,9 +315,11 @@ private:
 
         std::array<RegType, TargetSize> result { };
 
-        for (unsigned i = 0; i < TargetSize; i++) {
-            ASSERT(sourceArray[i] != static_cast<int32_t>(InfoTypeForReg<RegType>::InvalidIndex));
-            result[i] = sourceArray[i];
+        if (TargetSize > 0) {
+            for (unsigned i = 0; i < TargetSize; i++) {
+                ASSERT(sourceArray[i] != static_cast<int32_t>(InfoTypeForReg<RegType>::InvalidIndex));
+                result[i] = sourceArray[i];
+            }
         }
 
         return result;

I don't pretend to understand the code and I don't know what to do with them. I guess as long as we are instantiating these templates with TargetSize or NumberOfRegisters set to 0, we'll need to create separate template specializations to avoid the warning? I'd sooner just disable the warning with pragmas if it doesn't look like an actual bug.

[Michael Catanzaro]

I'm going to add IGNORE_WARNINGS macros, that's all I can really do here.

[Michael Catanzaro]

Created attachment 399130 [details]
Patch

[Mark Lam]

Can you try adding if constexpr checks around the for statements like so:

        if constexpr (!!NumberOfRegisters) {
            for (unsigned i = 0; i < NumberOfRegisters; ++i) {
                if (sources[i] != destinations[i])
                    pairs.append(std::make_pair(sources[i], destinations[i]));
            }
        }

        if constexpr (!!TargetSize) {
            for (unsigned i = 0; i < TargetSize; i++) {
                ASSERT(sourceArray[i] != static_cast<int32_t>(InfoTypeForReg<RegType>::InvalidIndex));
                result[i] = sourceArray[i];
            }
        }

See if that helps.  If it does, add a comment about why the if constexpr was added.

[Mark Lam]

Are any of the EWS using GCC10 yet?  If not, are there plans to migrate that way?   It's going to be difficult to keep these from continuing to manifest without the EWS.

[Darin Adler]

Comment on attachment 399130 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=399130&action=review

> Source/JavaScriptCore/jit/CCallHelpers.h:103
>          for (unsigned i = 0; i < NumberOfRegisters; ++i) {

Is this warning just about when NumberOfRegisters is 0?

[Darin Adler]

(In reply to Mark Lam from comment #3)
> Can you try adding if constexpr checks

We should try this!

[Michael Catanzaro]

Thanks for being responsive to my attempts to keep the GCC build nice and clean. :)

(In reply to Mark Lam from comment #4)
> Are any of the EWS using GCC10 yet?  If not, are there plans to migrate that
> way?   It's going to be difficult to keep these from continuing to manifest
> without the EWS.

I'll let Igalia answer that. afaik the current EWS just uses whatever GCC is provided by Debian stable. Having more EWS would certainly be nice.

Also note that EWS does not build with -Werror, so there's no way to notice warnings even on current compiler versions. I think changing that would catch probably 90% of GCC warnings before they're committed to trunk, since relatively few warnings are new warnings.

(In reply to Darin Adler from comment #5)
> > Source/JavaScriptCore/jit/CCallHelpers.h:103
> >          for (unsigned i = 0; i < NumberOfRegisters; ++i) {
> 
> Is this warning just about when NumberOfRegisters is 0?

Right.

(In reply to Mark Lam from comment #3)
> Can you try adding if constexpr checks around the for statements like so:

Oooh, didn't know about constexpr if. I think that worked, nice! Will let it continue building a while longer just to be certain.

[Michael Catanzaro]

Created attachment 399179 [details]
Patch

[Darin Adler]

Comment on attachment 399179 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=399179&action=review

> Source/JavaScriptCore/jit/CCallHelpers.h:103
> +        if constexpr (!!NumberOfRegisters) {

I don’t think we need the !!

> Source/JavaScriptCore/jit/CCallHelpers.h:320
> +        if constexpr (!!TargetSize) {

Ditto.

[Mark Lam]

Comment on attachment 399179 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=399179&action=review

>> Source/JavaScriptCore/jit/CCallHelpers.h:103
>> +        if constexpr (!!NumberOfRegisters) {
> 
> I don’t think we need the !!

I already tried without the !!.  Clang complains about ints not matching bools.  That's why I added it.

[Michael Catanzaro]

How about if constexpr (NumberOfRegisters > 0)? That's entirely equivalent and complies with WebKit code style.

The clearest thing to do would be to write '== 0', but our code style doesn't allow that.

[Mark Lam]

(In reply to Michael Catanzaro from comment #11)
> How about if constexpr (NumberOfRegisters > 0)? That's entirely equivalent
> and complies with WebKit code style.
> 
> The clearest thing to do would be to write '== 0', but our code style
> doesn't allow that.

IMHO, (NumberOfRegisters > 0) is fine or you can just leave it as (!!NumberOfRegisters).

[Michael Catanzaro]

Created attachment 399498 [details]
Patch for landing

[Michael Catanzaro]

Apparently the > 0 construction doesn't avoid the build warning...? Huh....

[Michael Catanzaro]

(In reply to Michael Catanzaro from comment #14)
> Apparently the > 0 construction doesn't avoid the build warning...? Huh....

Ah no, it probably does, I just reverted my local changes out of habit before starting a test build to check it. :P

[EWS]

Committed r261754: <https://trac.webkit.org/changeset/261754>

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

[Radar WebKit Bug Importer]

<rdar://problem/63284459>