Bug 161521 – jsc: fix cmake build missing symbol getPropertySlot

• Component: JavaScriptCore
• Status: RESOLVED FIXED

[JF Bastien]

The following patch moved things around: https://bugs.webkit.org/show_bug.cgi?id=161499

It seems to upset the cmake build with:

Undefined symbols for architecture x86_64:
  "JSC::JSObject::getPropertySlot(JSC::ExecState*, unsigned int, JSC::PropertySlot&)", referenced from:
      JSC::JSObject::get(JSC::ExecState*, JSC::PropertyName) const in IntlDateTimeFormat.cpp.o
      JSC::JSObject::get(JSC::ExecState*, JSC::PropertyName) const in IntlNumberFormat.cpp.o
  "JSC::JSObject::getNonIndexPropertySlot(JSC::ExecState*, JSC::PropertyName, JSC::PropertySlot&)", referenced from:
      JSC::JSObject::get(JSC::ExecState*, JSC::PropertyName) const in IntlDateTimeFormat.cpp.o
      JSC::JSObject::get(JSC::ExecState*, JSC::PropertyName) const in IntlNumberFormat.cpp.o
ld: symbol(s) not found for architecture x86_64

Sure enough, I nm through all the .o files and all references to getPropertySlot are U.

The non-template functions are huge anyways, it seems optimistic to me that they would *ever* be inlined.

[JF Bastien]

Created attachment 287710 [details]
patch

[Filip Pizlo]

Comment on attachment 287710 [details]
patch

But these are marked ALWAYS_INLINE.  I believe that there are a few places where we actually rely on them being inlined.

You should carefully test performance with such a change.  I'm happy to r+ if you've run JetStream, Speedometer, and PLT3.

[JF Bastien]

Created attachment 287713 [details]
patch

Fixed by including JSCInlines.h instead of moving things to .cpp file (Fil and others pointed out that this would have perf implications, etc).

This also fixes the cmake build.

[WebKit Commit Bot]

Attachment 287713 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/runtime/IntlDateTimeFormat.cpp:38:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/JavaScriptCore/runtime/IntlNumberFormat.cpp:38:  Alphabetical sorting problem.  [build/include_order] [4]
Total errors found: 2 in 3 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Saam Barati]

Comment on attachment 287713 [details]
patch

r=me
I'm going to let EWS build before cq+

[WebKit Commit Bot]

Comment on attachment 287713 [details]
patch

Clearing flags on attachment: 287713

Committed r205332: <http://trac.webkit.org/changeset/205332>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.