Bug 193073 – JSC should build successfully even with -DENABLE_UNIFIED_BUILDS=OFF

• Component: JavaScriptCore
• Status: RESOLVED FIXED

[Carlos Bentzen]

After the addition of ENABLE_UNIFIED_BUILDS, when trying to build JSC I get a lot of compilation errors due to missing #include's that are included transitively from other sources when bundled.

Compiler: GCC 8.2.1
Revision: 239568

[Carlos Bentzen]

Created attachment 358181 [details]
WIP

Still needs some other fixes and a few #include's need to be reviewed.

[Carlos Bentzen]

Created attachment 358558 [details]
Progress: only failing when linking testapi

After this patch we can compile up to libJavaScriptCore.so but when linking the testapi executable it fails with many missing symbols (all from *Inline.h files).

Yusuke, maybe you have some idea on why this is happening? Ps: does not happen with unified sources enabled.

lib/libJavaScriptCore.so.1.0.0: error: undefined reference to 'JSC::Wasm::SignatureInformation::singleton()'                                    
lib/libJavaScriptCore.so.1.0.0: error: undefined reference to 'JSC::StructureRareData::setCachedOwnKeys(JSC::VM&, JSC::JSImmutableButterfly*)'  
lib/libJavaScriptCore.so.1.0.0: error: undefined reference to 'JSC::StructureRareData::cachedOwnKeysIgnoringSentinel() const'                   
lib/libJavaScriptCore.so.1.0.0: error: undefined reference to 'JSC::StructureRareData::cachedOwnKeys() const'                                   
lib/libJavaScriptCore.so.1.0.0: error: undefined reference to 'JSC::JSValue::JSValue(JSC::JSValue::JSUndefinedTag)'                             
lib/libJavaScriptCore.so.1.0.0: error: undefined reference to 'JSC::JSValue::encode(JSC::JSValue)'                                            
lib/libJavaScriptCore.so.1.0.0: error: undefined reference to 'JSC::SlotVisitor::appendValuesHidden(JSC::WriteBarrierBase<JSC::Unknown, WTF::DumbValueTraits<JSC::Unknown> > con
st*, unsigned long)'                                                                                                                            
lib/libJavaScriptCore.so.1.0.0: error: undefined reference to 'JSC::JSValue::isNumber() const'                                                  
lib/libJavaScriptCore.so.1.0.0: error: undefined reference to 'JSC::JSValue::isInt32() const'                                                   
lib/libJavaScriptCore.so.1.0.0: error: undefined reference to 'void JSC::JIT::privateCompilePutByValWithCachedId<JSC::OpPutByValDirect>(JSC::ByValInfo*, JSC::ReturnAddressPtr,
JSC::PutKind, JSC::Identifier const&)'                                                                                                        
lib/libJavaScriptCore.so.1.0.0: error: undefined reference to 'void JSC::JIT::privateCompilePutByValWithCachedId<JSC::OpPutByVal>(JSC::ByValInfo*, JSC::ReturnAddressPtr, JSC::P
utKind, JSC::Identifier const&)'                                                                                                                
lib/libJavaScriptCore.so.1.0.0: error: undefined reference to 'void JSC::JIT::privateCompilePutByVal<JSC::OpPutByValDirect>(JSC::ByValInfo*, JSC::ReturnAddressPtr, JSC::JITArra
yMode)'                                                                                                                                         
lib/libJavaScriptCore.so.1.0.0: error: undefined reference to 'void JSC::JIT::privateCompilePutByVal<JSC::OpPutByVal>(JSC::ByValInfo*, JSC::ReturnAddressPtr, JSC::JITArrayMode)
'                                                                                                                                               
lib/libJavaScriptCore.so.1.0.0: error: undefined reference to 'JSC::ExecState::isStackOverflowFrame() const'                                    
lib/libJavaScriptCore.so.1.0.0: error: undefined reference to 'JSC::Strong<JSC::Unknown>::Strong(JSC::VM&, JSC::JSValue)'                     
lib/libJavaScriptCore.so.1.0.0: error: undefined reference to 'JSC::Strong<JSC::JSGlobalObject>::Strong(JSC::VM&, JSC::JSGlobalObject*)'        
lib/libJavaScriptCore.so.1.0.0: error: undefined reference to 'JSC::IsoSubspace::allocateNonVirtual(JSC::VM&, unsigned long, JSC::GCDeferralContext*, JSC::AllocationFailureMode
)'                                                                                                                                              
lib/libJavaScriptCore.so.1.0.0: error: undefined reference to 'JSC::RegisterID* JSC::BytecodeGenerator::emitEqualityOp<JSC::OpLess>(JSC::RegisterID*, JSC::RegisterID*, JSC::Reg
isterID*)'                                                                                                                                      
lib/libJavaScriptCore.so.1.0.0: error: undefined reference to 'JSC::RegisterID* JSC::BytecodeGenerator::emitEqualityOp<JSC::OpEq>(JSC::RegisterID*, JSC::RegisterID*, JSC::Regis
terID*)'                                                                                                                                      
lib/libJavaScriptCore.so.1.0.0: error: undefined reference to 'JSC::UnlinkedMetadataTable::addEntry(JSC::OpcodeID)'                             
lib/libJavaScriptCore.so.1.0.0: error: undefined reference to 'JSC::JSCell::structure(JSC::VM&) const'                                          
lib/libJavaScriptCore.so.1.0.0: error: undefined reference to 'JSC::JSValue::isNull() const'                                                    
lib/libJavaScriptCore.so.1.0.0: error: undefined reference to 'JSC::Structure::prototypeForLookup(JSC::JSGlobalObject*, JSC::JSCell*) const'    
lib/libJavaScriptCore.so.1.0.0: error: undefined reference to 'JSC::JSValue::isEmpty() const'                                                 
lib/libJavaScriptCore.so.1.0.0: error: undefined reference to 'JSC::JSValue::decode(long)'                                                      
lib/libJavaScriptCore.so.1.0.0: error: undefined reference to 'JSC::HeapCell::vm() const'                                                       
lib/libJavaScriptCore.so.1.0.0: error: undefined reference to 'JSC::JSValue::JSValue(JSC::JSCell*)'                                                                       
lib/libJavaScriptCore.so.1.0.0: error: undefined reference to 'void JSC::SlotVisitor::append<JSC::CodeBlock, WTF::DumbPtrTraits<JSC::CodeBlock> >(JSC::WriteBarrierBase<JSC::CodeBlock, WTF::DumbPtrTraits<JSC::CodeBlock> > const&)'           
lib/libJavaScriptCore.so.1.0.0: error: undefined reference to 'JSC::SlotVisitor::appendUnbarriered(JSC::JSCell*)'
lib/libJavaScriptCore.so.1.0.0: error: undefined reference to 'JSC::SlotVisitor::vm()'
lib/libJavaScriptCore.so.1.0.0: error: undefined reference to 'JSC::Heap::writeBarrier(JSC::JSCell const*, JSC::JSCell*)'
lib/libJavaScriptCore.so.1.0.0: error: undefined reference to 'JSC::JSValue::asCell() const'
lib/libJavaScriptCore.so.1.0.0: error: undefined reference to 'JSC::JSValue::operator bool() const'
lib/libJavaScriptCore.so.1.0.0: error: undefined reference to 'JSC::Heap::isMarked(void const*)'
lib/libJavaScriptCore.so.1.0.0: error: undefined reference to 'JSC::Heap::mutatorFence()'
lib/libJavaScriptCore.so.1.0.0: error: undefined reference to 'JSC::Heap::vm() const'
lib/libJavaScriptCore.so.1.0.0: error: undefined reference to 'JSC::Structure::prototypeForLookup(JSC::JSGlobalObject*) const'
lib/libJavaScriptCore.so.1.0.0: error: undefined reference to 'JSC::Identifier JSC::Identifier::fromString<40u>(JSC::VM*, char const (&) [40u])'
lib/libJavaScriptCore.so.1.0.0: error: undefined reference to 'JSC::Identifier JSC::Identifier::fromString<32u>(JSC::VM*, char const (&) [32u])'
lib/libJavaScriptCore.so.1.0.0: error: undefined reference to 'JSC::Identifier JSC::Identifier::fromString<34u>(JSC::VM*, char const (&) [34u])'
lib/libJavaScriptCore.so.1.0.0: error: undefined reference to 'JSC::Identifier JSC::Identifier::fromString<49u>(JSC::VM*, char const (&) [49u])'
lib/libJavaScriptCore.so.1.0.0: error: undefined reference to 'JSC::Identifier JSC::Identifier::fromString<29u>(JSC::VM*, char const (&) [29u])'
lib/libJavaScriptCore.so.1.0.0: error: undefined reference to 'JSC::Identifier JSC::Identifier::fromString<36u>(JSC::VM*, char const (&) [36u])'
lib/libJavaScriptCore.so.1.0.0: error: undefined reference to 'JSC::Identifier JSC::Identifier::fromString<24u>(JSC::VM*, char const (&) [24u])'
lib/libJavaScriptCore.so.1.0.0: error: undefined reference to 'JSC::Identifier JSC::Identifier::fromString<3u>(JSC::VM*, char const (&) [3u])'
lib/libJavaScriptCore.so.1.0.0: error: undefined reference to 'JSC::Identifier JSC::Identifier::fromString<41u>(JSC::VM*, char const (&) [41u])'
lib/libJavaScriptCore.so.1.0.0: error: undefined reference to 'JSC::Identifier JSC::Identifier::fromString<39u>(JSC::VM*, char const (&) [39u])'
lib/libJavaScriptCore.so.1.0.0: error: undefined reference to 'JSC::Identifier JSC::Identifier::fromString<16u>(JSC::VM*, char const (&) [16u])'
lib/libJavaScriptCore.so.1.0.0: error: undefined reference to 'JSC::Identifier JSC::Identifier::fromString<13u>(JSC::VM*, char const (&) [13u])'
lib/libJavaScriptCore.so.1.0.0: error: undefined reference to 'JSC::Identifier JSC::Identifier::fromString<12u>(JSC::VM*, char const (&) [12u])'
lib/libJavaScriptCore.so.1.0.0: error: undefined reference to 'JSC::Identifier JSC::Identifier::fromString<9u>(JSC::VM*, char const (&) [9u])'
lib/libJavaScriptCore.so.1.0.0: error: undefined reference to 'JSC::Identifier JSC::Identifier::fromString<14u>(JSC::VM*, char const (&) [14u])'
lib/libJavaScriptCore.so.1.0.0: error: undefined reference to 'JSC::Identifier JSC::Identifier::fromString<10u>(JSC::VM*, char const (&) [10u])'
lib/libJavaScriptCore.so.1.0.0: error: undefined reference to 'JSC::Identifier JSC::Identifier::fromString<8u>(JSC::VM*, char const (&) [8u])'
lib/libJavaScriptCore.so.1.0.0: error: undefined reference to 'JSC::Identifier JSC::Identifier::fromString<28u>(JSC::VM*, char const (&) [28u])'
lib/libJavaScriptCore.so.1.0.0: error: undefined reference to 'JSC::Identifier JSC::Identifier::fromString<31u>(JSC::VM*, char const (&) [31u])'
lib/libJavaScriptCore.so.1.0.0: error: undefined reference to 'JSC::Identifier JSC::Identifier::fromString<15u>(JSC::VM*, char const (&) [15u])'
lib/libJavaScriptCore.so.1.0.0: error: undefined reference to 'JSC::Identifier JSC::Identifier::fromString<17u>(JSC::VM*, char const (&) [17u])'
lib/libJavaScriptCore.so.1.0.0: error: undefined reference to 'JSC::Identifier JSC::Identifier::fromString<5u>(JSC::VM*, char const (&) [5u])'
lib/libJavaScriptCore.so.1.0.0: error: undefined reference to 'JSC::Identifier JSC::Identifier::fromString<11u>(JSC::VM*, char const (&) [11u])'
lib/libJavaScriptCore.so.1.0.0: error: undefined reference to 'JSC::Identifier JSC::Identifier::fromString<25u>(JSC::VM*, char const (&) [25u])'
lib/libJavaScriptCore.so.1.0.0: error: undefined reference to 'JSC::Identifier JSC::Identifier::fromString<27u>(JSC::VM*, char const (&) [27u])'
lib/libJavaScriptCore.so.1.0.0: error: undefined reference to 'JSC::Identifier JSC::Identifier::fromString<22u>(JSC::VM*, char const (&) [22u])'
lib/libJavaScriptCore.so.1.0.0: error: undefined reference to 'JSC::Identifier JSC::Identifier::fromString<23u>(JSC::VM*, char const (&) [23u])'
lib/libJavaScriptCore.so.1.0.0: error: undefined reference to 'JSC::Identifier JSC::Identifier::fromString<6u>(JSC::VM*, char const (&) [6u])'
lib/libJavaScriptCore.so.1.0.0: error: undefined reference to 'JSC::Identifier JSC::Identifier::fromString<7u>(JSC::VM*, char const (&) [7u])'
lib/libJavaScriptCore.so.1.0.0: error: undefined reference to 'JSC::Identifier JSC::Identifier::fromString<26u>(JSC::VM*, char const (&) [26u])'
lib/libJavaScriptCore.so.1.0.0: error: undefined reference to 'JSC::Identifier JSC::Identifier::fromString<20u>(JSC::VM*, char const (&) [20u])'
lib/libJavaScriptCore.so.1.0.0: error: undefined reference to 'JSC::Identifier JSC::Identifier::fromString<18u>(JSC::VM*, char const (&) [18u])'
lib/libJavaScriptCore.so.1.0.0: error: undefined reference to 'JSC::Identifier JSC::Identifier::fromString<21u>(JSC::VM*, char const (&) [21u])'
lib/libJavaScriptCore.so.1.0.0: error: undefined reference to 'JSC::Identifier JSC::Identifier::fromString<4u>(JSC::VM*, char const (&) [4u])'
lib/libJavaScriptCore.so.1.0.0: error: undefined reference to 'JSC::Identifier JSC::Identifier::fromString<19u>(JSC::VM*, char const (&) [19u])'
lib/libJavaScriptCore.so.1.0.0: error: undefined reference to 'JSC::Identifier JSC::Identifier::fromString<33u>(JSC::VM*, char const (&) [33u])'
Source/JavaScriptCore/shell/CMakeFiles/testapi.dir/__/API/tests/testapi.cpp.o:testapi.cpp:function TestAPI::basicSymbol(): error: undefined reference to 'JSValueIsSymbol'
Source/JavaScriptCore/shell/CMakeFiles/testapi.dir/__/API/tests/testapi.cpp.o:testapi.cpp:function WTF::SharedTaskFunctor<void (TestAPI&), testCAPIViaCpp::{lambda(TestAPI&)#2}>
::run(TestAPI&): error: undefined reference to 'JSValueIsSymbol'

[Fujii Hironori]

This is the reason I recommended using 'inline' specifier (Bug 191626 Comment 13).

[Yusuke Suzuki]

(In reply to Fujii Hironori from comment #3)
> This is the reason I recommended using 'inline' specifier (Bug 191626
> Comment 13).

Actually this is interesting. Can you post the discussion about it in webkit-dev mailing list?
If we need to change the Coding Style Guide, I think webkit-dev is the best place to discuss it.

[Fujii Hironori]

Got it. I'd like to create a proposal patch before having a discussion to make certain.

[Fujii Hironori]

It turned out it isn't easy as I expected (Bug 193481). Please ignore my comments. Sorry for disturbing.

[Ross Kirsling]

Looks like the linking errors are mostly due to the following:

1. <path>/a.h:   templated method is declared
2. <path>/a.cpp: templated method is defined and called with template param Foo
3. <path>/b.cpp: templated method is called with template param Bar

Which happens to work when unified sources are enabled because the implementation files are adjacent. 

Steph on our team noticed that technically one could add the following line after the method definition in a.cpp: 

>  template ... SomeClass::someMethod<Bar>(...);
But I'm assuming this would be less desirable than transplanting the definition to a more appropriate location. :P

[Ross Kirsling]

Now, ALWAYS_INLINE seems to be of no use, but for something like...
> LocalAllocator.cpp.obj : error LNK2019: unresolved external symbol "public: class JSC::HeapCell * __cdecl JSC::FreeList::allocate<class <lambda_8437a926922bdf66125377eabab8b83d> >(class <lambda_8437a926922bdf66125377eabab8b83d> const &)" (??$allocate@V<lambda_8437a926922bdf66125377eabab8b83d>@@@FreeList@JSC@@QEAAPEAVHeapCell@1@AEBV@@@Z) referenced in function "private: void * __cdecl JSC::LocalAllocator::tryAllocateIn(class JSC::MarkedBlock::Handle *)" (?tryAllocateIn@LocalAllocator@JSC@@AEAAPEAXPEAVHandle@MarkedBlock@2@@Z)
...we could just move the definition to the declaration site:
https://github.com/WebKit/webkit/blob/master/Source/JavaScriptCore/heap/FreeListInlines.h#L33-L50
https://github.com/WebKit/webkit/blob/master/Source/JavaScriptCore/heap/FreeList.h#L72-L73

But this gets kind of out of hand for something like...
> JITOperations.cpp.obj : error LNK2019: unresolved external symbol "private: void __cdecl JSC::JIT::privateCompilePutByVal<struct JSC::OpPutByVal>(class JSC::ConcurrentJSLocker const &,struct JSC::ByValInfo *,class JSC::ReturnAddressPtr,enum JSC::JITArrayMode)" (??$privateCompilePutByVal@UOpPutByVal@JSC@@@JIT@JSC@@AEAAXAEBVConcurrentJSLocker@1@PEAUByValInfo@1@VReturnAddressPtr@1@W4JITArrayMode@1@@Z) referenced in function "public: static void __cdecl JSC::JIT::compilePutByVal(class JSC::ConcurrentJSLocker const &,class JSC::VM *,class JSC::CodeBlock *,struct JSC::ByValInfo *,class JSC::ReturnAddressPtr,enum JSC::JITArrayMode)" (?compilePutByVal@JIT@JSC@@SAXAEBVConcurrentJSLocker@2@PEAVVM@2@PEAVCodeBlock@2@PEAUByValInfo@2@VReturnAddressPtr@2@W4JITArrayMode@2@@Z)
...in which case moving privateCompilePutByVal from JITPropertyAccess.cpp to JIT.h leads to a pretty severe domino effect. :(

[Ross Kirsling]

Created attachment 366870 [details]
Working solution (sufficient for MSVC but not clang)

[EWS Watchlist]

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


ERROR: Source/JavaScriptCore/API/JSObjectRef.h:706:  The parameter name "value" adds no information, so it should be removed.  [readability/parameter_name] [5]
Total errors found: 1 in 28 files


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

[Ross Kirsling]

This should be a working solution.

The first few changes described in the ChangeLog are likely unacceptable as-is, but hopefully this suffices to determine what we should actually do.

[Keith Miller]

Comment on attachment 366870 [details]
Working solution (sufficient for MSVC but not clang)

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

> Source/JavaScriptCore/ChangeLog:11
> +        testapi.cpp complains that it doesn't know about JSValueMakeSymbol / JSValueIsSymbol;
> +        moving their declarations evidently makes the problem go away.

That's bizarre. Can you compile with -E in clang and see if they are there?

[Ross Kirsling]

Comment on attachment 366870 [details]
Working solution (sufficient for MSVC but not clang)

Forgot to mention that the current patch is a working solution *for WinCairo*.

I'm unaware of a way to disable unified builds via Xcode, but since JSCOnly is Cmake-based, we can do this on Mac:
> Tools/Scripts/build-jsc --jsc-only --cmakeargs="-DENABLE_UNIFIED_BUILDS=OFF"
As it turns out, the Mac situation is far more dire, with another 50+ "missing" symbols defined in *Inlines.h. 😰

[Ross Kirsling]

Hmm, apparently even WinCairo behaves similarly (44 additional unresolved externals) when building with clang-cl.
(Funny that MSVC is easier to please in this scenario, though that may not actually be a good thing.)

---

(In reply to Keith Miller from comment #12)
> That's bizarre. Can you compile with -E in clang and see if they are there?

I can look into this, though it's slightly tricky as that particular issue only appeared after I addressed all of the other unresolved externals.

[Ross Kirsling]

Another interesting point (which I didn't see earlier because MSVC is not as kind):

> ..\..\Source\JavaScriptCore\heap/FreeList.h(73,15):  error: function 'JSC::FreeList::allocate<(lambda at ..\..\Source\JavaScriptCore\heap\LocalAllocator.cpp:235:9)>' is used but not defined in this translation unit, and cannot be defined in any other translation unit because its type does not have linkage
>     HeapCell* allocate(const Func& slowPath);
>               ^
> ..\..\Source\JavaScriptCore\heap\LocalAllocator.cpp(234,31):  note: used here
>     void* result = m_freeList.allocate(
>                               ^

This particular case is an outright clang error since it's definitionally broken (as it's impossible to "prepare" an implementation when the template parameter is a lambda).

So I guess the overall task is what to do about the dozens of other cases that aren't *definitionally* broken but have the exact same translation unit spillage problem.

[Ross Kirsling]

(In reply to Ross Kirsling from comment #14)
> (In reply to Keith Miller from comment #12)
> > > Source/JavaScriptCore/ChangeLog:11
> > > +        testapi.cpp complains that it doesn't know about JSValueMakeSymbol / JSValueIsSymbol;
> > > +        moving their declarations evidently makes the problem go away.
> >
> > That's bizarre. Can you compile with -E in clang and see if they are there?
> 
> I can look into this, though it's slightly tricky as that particular issue
> only appeared after I addressed all of the other unresolved externals.

Hmm, this case is probably the same idea as the others after all.

For instance, if the linker can't see this method definition:
https://github.com/WebKit/webkit/blob/master/Source/JavaScriptCore/runtime/IdentifierInlines.h#L96-L100

Then we *could* move it to the declaration (as I've done in the current patch):
https://github.com/WebKit/webkit/blob/master/Source/JavaScriptCore/runtime/Identifier.h#L113-L114

But it also suffices just to move it under the class declaration:
https://github.com/WebKit/webkit/blob/master/Source/JavaScriptCore/runtime/Identifier.h#L205

And so in both cases, the problem occurs when the method definition is not in the header where the declaration occurs, but rather in a second header that includes the first.

Refocusing on Identifier::fromString then, if I compile Identifier.cpp and BuiltinNames.cpp with -E as you suggested, sure enough, Identifier.cpp.o contains the source of IdentifierInlines.h but BuiltinNames.cpp.o does not.

[Ross Kirsling]

Created attachment 366897 [details]
Patch

[Ross Kirsling]

So I guess the lesson learned here is "when you forget to include *Inlines.h, you get linking errors and not compiler errors".

The two legitimately tricky cases are of the type I initially mentioned (template method defined in a.cpp but consumed in b.cpp), but the rest all turned out to be missing includes after all.

[Ross Kirsling]

Created attachment 366909 [details]
Patch

[Ross Kirsling]

Somehow, the previous patch was sufficient for Mac (JSCOnly) but not enough to please WinCairo clang-cl.

This one should cover all the bases.

[Ross Kirsling]

Created attachment 366946 [details]
Patch

[Ross Kirsling]

Rebased since bug 196647 was landed without noticing this one.

[Yusuke Suzuki]

Comment on attachment 366946 [details]
Patch

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

Can we have a way to avoid "JSCInlines.h" repeatedly? It means that one of the header in JSCInlines.h is modified, so many C++ source code will be recompiled :(

> Source/JavaScriptCore/b3/B3OptimizeAssociativeExpressionTrees.cpp:34
>  #include "B3InsertionSet.h"

When we include XXXInlines.h, we omit XXX.h include.

> Source/JavaScriptCore/runtime/Operations.h:27
>  #include "JSCJSValue.h"

Ditto.

[Ross Kirsling]

(In reply to Yusuke Suzuki from comment #23)
> Comment on attachment 366946 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=366946&action=review
> 
> Can we have a way to avoid "JSCInlines.h" repeatedly? It means that one of
> the header in JSCInlines.h is modified, so many C++ source code will be
> recompiled :(

That's a great question for which I don't have an answer, but I noticed that JSCInlines.h itself claims that
> It's good style to make sure that every .cpp file includes JSCInlines.h.
(https://github.com/WebKit/webkit/blob/master/Source/JavaScriptCore/runtime/JSCInlines.h#L28-L30)

Now, when a .cpp file doesn't actually use the symbol that its .cpp.o is missing, I tried to include the *Inlines.h from the header that does use the symbol (most notably, by including JSCJSValueInlines.h from Operations.h, I avoided having to add includes to about 20 DFG*.cpp files), but if this is the "wrong way" then please let me know. :)

[Ross Kirsling]

Two further points:

- Since I interpreted the quote above as a JSC best practice, I intentionally included JSCInlines.h instead of listing more specific *Inlines.h includes in .cpp files. Happy to reverse this approach if it's preferred.

- Importantly, I avoided including JSCInlines.h in .h files and just included the more specific *Inlines.h instead (such as when including JSCJSValueInlines.h from Operations.h as mentioned above).

[Ross Kirsling]

Created attachment 367011 [details]
Patch

[Keith Miller]

Comment on attachment 367011 [details]
Patch

r=me.

[WebKit Commit Bot]

Comment on attachment 367011 [details]
Patch

Clearing flags on attachment: 367011

Committed r244088: <https://trac.webkit.org/changeset/244088>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/49743722>