Bug 211783 – -Wsign-compare warnings in FTLLowerDFGToB3.cpp and DFGSpeculativeJIT.cpp

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

[Michael Catanzaro]

This fixes -Wsign-compare warnings introduced in bug #206182. See https://bugs.webkit.org/show_bug.cgi?id=206182#c47.

Note that I'm only submitting a *simple* solution. JSValue::BigInt32Mask is currently a negative-valued signed integer, and I'm skeptical that that's desirable, since unsigned is usually better to represent bit patterns. I'm not volunteering to try changing that, though.

[Michael Catanzaro]

Created attachment 399134 [details]
Patch

[Mark Lam]

Comment on attachment 399134 [details]
Patch

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

> Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:3600
> +        static_assert(JSValue::BigInt32Mask == static_cast<int64_t>(0xfffe000000000012));

Would using a "ll" suffix suffice?  0xfffe000000000012ll

> Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:3346
> +            static_assert(JSValue::BigInt32Mask == static_cast<int64_t>(0xfffe000000000012));

Ditto.

[Darin Adler]

Comment on attachment 399134 [details]
Patch

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

r=me because I don’t see a better solution

>> Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:3600
>> +        static_assert(JSValue::BigInt32Mask == static_cast<int64_t>(0xfffe000000000012));
> 
> Would using a "ll" suffix suffice?  0xfffe000000000012ll

I think that won’t work because the value doesn’t fit in a 64-bit long long, and so the language makes it unsigned long long in that case. I consulted https://en.cppreference.com/w/cpp/language/integer_literal to explore this.

[EWS]

Committed r261564: <https://trac.webkit.org/changeset/261564>

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

[Radar WebKit Bug Importer]

<rdar://problem/63143618>

[Michael Catanzaro]

(In reply to Darin Adler from comment #3)
> I think that won’t work because the value doesn’t fit in a 64-bit long long,
> and so the language makes it unsigned long long in that case. I consulted
> https://en.cppreference.com/w/cpp/language/integer_literal to explore this.

Exactly. I had consulted the same page to figure out how integer literals work. Learning is good!