Bug 224616 – Refactor inline functions out of HashMapImpl.h into HashMapImplInlines.h.

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

[Mark Lam]

Also do the same for clients of HashMapImpl that require similar refactoring.  This fixes the #include of JSCJSValueInlines.h in HashMapImpl.h, as well as makes it easier to use other inline functions from other classes in the implementation of HashMapImpl's inline functions in the future.

[Radar WebKit Bug Importer]

<rdar://problem/76713709>

[Mark Lam]

Created attachment 426121 [details]
proposed patch.

[Yusuke Suzuki]

Comment on attachment 426121 [details]
proposed patch.

r=me

[Darin Adler]

Comment on attachment 426121 [details]
proposed patch.

Never noticed when this first was introduced: Seems unfortunate that WTF has WTF::HashMap and JavaScriptCore has unrelated JSC::HashMapImpl. Might be worth thinking about future name changes to clarify the relationship/distinction

[Mark Lam]

Thanks for the review.  Landed in r276039: <http://trac.webkit.org/r276039>.

[Michael Catanzaro]

New warnings:

[1215/5456] Building CXX object Source/JavaScriptCore/CMa...edSources/unified-sources/UnifiedSource-f0a787a9-11.cpp.o
In file included from ../../Source/JavaScriptCore/runtime/JSMap.h:28,
                 from ../../Source/JavaScriptCore/bytecode/SpeculatedType.cpp:39,
                 from JavaScriptCore/DerivedSources/unified-sources/UnifiedSource-f0a787a9-11.cpp:5:
../../Source/JavaScriptCore/runtime/HashMapImpl.h:233:27: warning: ‘bool JSC::areKeysEqual(JSC::JSGlobalObject*, JSC::JSValue, JSC::JSValue)’ declared ‘static’ but never defined [-Wunused-function]
  233 | ALWAYS_INLINE static bool areKeysEqual(JSGlobalObject*, JSValue, JSValue);
      |                           ^~~~~~~~~~~~
../../Source/JavaScriptCore/runtime/HashMapImpl.h:238:31: warning: ‘uint32_t JSC::wangsInt64Hash(uint64_t)’ declared ‘static’ but never defined [-Wunused-function]
  238 | static ALWAYS_INLINE uint32_t wangsInt64Hash(uint64_t key);
      |

I think we could simply remove the declarations of these functions from HashMapImpl.h. If they're needed, better include HashMapImplInlines.h, yes?

[Mark Lam]

(In reply to Michael Catanzaro from comment #6)
> New warnings:
> 
> [1215/5456] Building CXX object
> Source/JavaScriptCore/CMa...edSources/unified-sources/UnifiedSource-f0a787a9-
> 11.cpp.o
> In file included from ../../Source/JavaScriptCore/runtime/JSMap.h:28,
>                  from
> ../../Source/JavaScriptCore/bytecode/SpeculatedType.cpp:39,
>                  from
> JavaScriptCore/DerivedSources/unified-sources/UnifiedSource-f0a787a9-11.cpp:
> 5:
> ../../Source/JavaScriptCore/runtime/HashMapImpl.h:233:27: warning: ‘bool
> JSC::areKeysEqual(JSC::JSGlobalObject*, JSC::JSValue, JSC::JSValue)’
> declared ‘static’ but never defined [-Wunused-function]
>   233 | ALWAYS_INLINE static bool areKeysEqual(JSGlobalObject*, JSValue,
> JSValue);
>       |                           ^~~~~~~~~~~~
> ../../Source/JavaScriptCore/runtime/HashMapImpl.h:238:31: warning: ‘uint32_t
> JSC::wangsInt64Hash(uint64_t)’ declared ‘static’ but never defined
> [-Wunused-function]
>   238 | static ALWAYS_INLINE uint32_t wangsInt64Hash(uint64_t key);
>       |
> 
> I think we could simply remove the declarations of these functions from
> HashMapImpl.h. If they're needed, better include HashMapImplInlines.h, yes?

Looking at this again, I think these 2 functions are only used by clients in HeapMapImplInlines.h.  So, yes, I think we can simply remove these declarations from HashMapImpl.h.

[Yusuke Suzuki]

Committed r276217 (236699@main): <https://commits.webkit.org/236699@main>