Bug 110024 – Move all Structure out-of-line inline methods to StructureInlines.h

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

[Filip Pizlo]

Already rubber stamped by Mark Hahnenberg.  But I'll put it up to EWS.

[Filip Pizlo]

Created attachment 188723 [details]
patch for landing

[WebKit Review Bot]

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

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/ChangeLog', u'Source/JavaScriptCore/GNUmakefile.list.am', u'Source/JavaScriptCore/JavaScriptCore.vcproj/JavaScriptCore/JavaScriptCore.vcproj', u'Source/JavaScriptCore/JavaScriptCore.xcodeproj/project.pbxproj', u'Source/JavaScriptCore/bytecode/CodeBlock.h', u'Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp', u'Source/JavaScriptCore/heap/HandleStack.cpp', u'Source/JavaScriptCore/runtime/DatePrototype.cpp', u'Source/JavaScriptCore/runtime/JSGlobalObject.cpp', u'Source/JavaScriptCore/runtime/JSGlobalObject.h', u'Source/JavaScriptCore/runtime/Operations.h', u'Source/JavaScriptCore/runtime/Structure.h', u'Source/JavaScriptCore/runtime/StructureInlines.h', u'Source/JavaScriptCore/runtime/StructureRareData.cpp']" exit_code: 1
Source/JavaScriptCore/runtime/StructureInlines.h:42:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
Source/JavaScriptCore/runtime/StructureInlines.h:60:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
Source/JavaScriptCore/runtime/StructureInlines.h:174:  Missing space before ( in while(  [whitespace/parens] [5]
Total errors found: 3 in 11 files


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

[Early Warning System Bot]

Comment on attachment 188723 [details]
patch for landing

Attachment 188723 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/16593722

[Early Warning System Bot]

Comment on attachment 188723 [details]
patch for landing

Attachment 188723 [details] did not pass qt-wk2-ews (qt):
Output: http://queues.webkit.org/results/16590669

[Build Bot]

Comment on attachment 188723 [details]
patch for landing

Attachment 188723 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://queues.webkit.org/results/16586725

[EFL EWS Bot]

Comment on attachment 188723 [details]
patch for landing

Attachment 188723 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/16593728

[kov's GTK+ EWS bot]

Comment on attachment 188723 [details]
patch for landing

Attachment 188723 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/16590674

[Filip Pizlo]

Created attachment 188756 [details]
work in progress

It turns out that this is harder than it looked.

[Filip Pizlo]

Created attachment 188759 [details]
the patch

[Build Bot]

Comment on attachment 188759 [details]
the patch

Attachment 188759 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://queues.webkit.org/results/16595861

[Filip Pizlo]

Created attachment 188777 [details]
the patch

Rebased. (A semantically but not textually conflicting change was the reason for the wk2 failure, above.)

[WebKit Review Bot]

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

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/API/JSCallbackFunction.cpp', u'Source/JavaScriptCore/CMakeLists.txt', u'Source/JavaScriptCore/ChangeLog', u'Source/JavaScriptCore/GNUmakefile.list.am', u'Source/JavaScriptCore/JavaScriptCore.vcproj/JavaScriptCore/JavaScriptCore.vcproj', u'Source/JavaScriptCore/JavaScriptCore.xcodeproj/project.pbxproj', u'Source/JavaScriptCore/Target.pri', u'Source/JavaScriptCore/bytecode/CodeBlock.h', u'Source/JavaScriptCore/bytecode/EvalCodeCache.h', u'Source/JavaScriptCore/bytecode/SamplingTool.h', u'Source/JavaScriptCore/bytecode/UnlinkedCodeBlock.cpp', u'Source/JavaScriptCore/bytecode/UnlinkedCodeBlock.h', u'Source/JavaScriptCore/bytecompiler/BytecodeGenerator.h', u'Source/JavaScriptCore/bytecompiler/Label.h', u'Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp', u'Source/JavaScriptCore/dfg/DFGByteCodeParser.h', u'Source/JavaScriptCore/dfg/DFGFPRInfo.h', u'Source/JavaScriptCore/dfg/DFGRegisterBank.h', u'Source/JavaScriptCore/heap/HandleStack.cpp', u'Source/JavaScriptCore/jit/JITWriteBarrier.h', u'Source/JavaScriptCore/parser/Nodes.h', u'Source/JavaScriptCore/parser/Parser.h', u'Source/JavaScriptCore/parser/ParserError.h', u'Source/JavaScriptCore/parser/ParserModes.h', u'Source/JavaScriptCore/parser/SourceProvider.cpp', u'Source/JavaScriptCore/parser/SourceProvider.h', u'Source/JavaScriptCore/runtime/ArrayPrototype.cpp', u'Source/JavaScriptCore/runtime/DatePrototype.cpp', u'Source/JavaScriptCore/runtime/Executable.h', u'Source/JavaScriptCore/runtime/JSGlobalObject.cpp', u'Source/JavaScriptCore/runtime/JSGlobalObject.h', u'Source/JavaScriptCore/runtime/Operations.h', u'Source/JavaScriptCore/runtime/Structure.cpp', u'Source/JavaScriptCore/runtime/Structure.h', u'Source/JavaScriptCore/runtime/StructureInlines.h', u'Source/JavaScriptCore/runtime/StructureRareData.cpp', u'Source/WebCore/ChangeLog', u'Source/WebCore/ForwardingHeaders/parser/SourceProviderCache.h', u'Source/WebCore/loader/cache/CachedScript.cpp']" exit_code: 1
Source/JavaScriptCore/runtime/StructureInlines.h:41:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
Source/JavaScriptCore/runtime/StructureInlines.h:59:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
Source/JavaScriptCore/runtime/StructureInlines.h:168:  Missing space before ( in while(  [whitespace/parens] [5]
Source/JavaScriptCore/jit/JITWriteBarrier.h:34:  Alphabetical sorting problem.  [build/include_order] [4]
Source/JavaScriptCore/dfg/DFGFPRInfo.h:31:  Alphabetical sorting problem.  [build/include_order] [4]
Source/JavaScriptCore/parser/SourceProvider.h:40:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Source/JavaScriptCore/parser/SourceProvider.h:46:  The parameter name "cache" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/bytecompiler/Label.h:40:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Total errors found: 8 in 35 files


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

[Filip Pizlo]

Created attachment 188795 [details]
patch for landing

I think I figured it out.

[WebKit Review Bot]

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

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/API/JSCallbackFunction.cpp', u'Source/JavaScriptCore/CMakeLists.txt', u'Source/JavaScriptCore/ChangeLog', u'Source/JavaScriptCore/GNUmakefile.list.am', u'Source/JavaScriptCore/JavaScriptCore.vcproj/JavaScriptCore/JavaScriptCore.vcproj', u'Source/JavaScriptCore/JavaScriptCore.xcodeproj/project.pbxproj', u'Source/JavaScriptCore/Target.pri', u'Source/JavaScriptCore/bytecode/CodeBlock.h', u'Source/JavaScriptCore/bytecode/EvalCodeCache.h', u'Source/JavaScriptCore/bytecode/SamplingTool.h', u'Source/JavaScriptCore/bytecode/UnlinkedCodeBlock.cpp', u'Source/JavaScriptCore/bytecode/UnlinkedCodeBlock.h', u'Source/JavaScriptCore/bytecompiler/BytecodeGenerator.h', u'Source/JavaScriptCore/bytecompiler/Label.h', u'Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp', u'Source/JavaScriptCore/dfg/DFGByteCodeParser.h', u'Source/JavaScriptCore/dfg/DFGFPRInfo.h', u'Source/JavaScriptCore/dfg/DFGRegisterBank.h', u'Source/JavaScriptCore/heap/HandleStack.cpp', u'Source/JavaScriptCore/jit/JITWriteBarrier.h', u'Source/JavaScriptCore/parser/Nodes.h', u'Source/JavaScriptCore/parser/Parser.h', u'Source/JavaScriptCore/parser/ParserError.h', u'Source/JavaScriptCore/parser/ParserModes.h', u'Source/JavaScriptCore/parser/SourceProvider.cpp', u'Source/JavaScriptCore/parser/SourceProvider.h', u'Source/JavaScriptCore/runtime/ArrayPrototype.cpp', u'Source/JavaScriptCore/runtime/DatePrototype.cpp', u'Source/JavaScriptCore/runtime/Executable.h', u'Source/JavaScriptCore/runtime/JSGlobalObject.cpp', u'Source/JavaScriptCore/runtime/JSGlobalObject.h', u'Source/JavaScriptCore/runtime/Operations.h', u'Source/JavaScriptCore/runtime/Structure.cpp', u'Source/JavaScriptCore/runtime/Structure.h', u'Source/JavaScriptCore/runtime/StructureInlines.h', u'Source/JavaScriptCore/runtime/StructureRareData.cpp', u'Source/WebCore/ChangeLog', u'Source/WebCore/ForwardingHeaders/parser/SourceProviderCache.h', u'Source/WebCore/loader/cache/CachedScript.cpp']" exit_code: 1
Source/JavaScriptCore/parser/SourceProvider.h:40:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Source/JavaScriptCore/bytecompiler/Label.h:40:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Total errors found: 2 in 35 files


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

[Filip Pizlo]

Landed in http://trac.webkit.org/changeset/143147

[Csaba Osztrogonác]

(In reply to comment #15)
> Landed in http://trac.webkit.org/changeset/143147

It broke JSVALUE32_64 builds:
In file included from /mnt/raptor2/slaves/qt5-linux-armv7-release/build/Source/JavaScriptCore/dfg/DFGOSREntry.cpp:34:0:
/mnt/raptor2/slaves/qt5-linux-armv7-release/build/Source/JavaScriptCore/jit/JIT.h:549:77: error: 'ResultType' has not been declared
/mnt/raptor2/slaves/qt5-linux-armv7-release/build/Source/JavaScriptCore/jit/JIT.h:550:77: error: 'ResultType' has not been declared

The problem was the following lines in JIT.h:
        void emitAdd32Constant(unsigned dst, unsigned op, int32_t constant, ResultType opType);
        void emitSub32Constant(unsigned dst, unsigned op, int32_t constant, ResultType opType);

Buildfix landed in https://trac.webkit.org/changeset/143154

[Filip Pizlo]

(In reply to comment #16)
> (In reply to comment #15)
> > Landed in http://trac.webkit.org/changeset/143147
> 
> It broke JSVALUE32_64 builds:
> In file included from /mnt/raptor2/slaves/qt5-linux-armv7-release/build/Source/JavaScriptCore/dfg/DFGOSREntry.cpp:34:0:
> /mnt/raptor2/slaves/qt5-linux-armv7-release/build/Source/JavaScriptCore/jit/JIT.h:549:77: error: 'ResultType' has not been declared
> /mnt/raptor2/slaves/qt5-linux-armv7-release/build/Source/JavaScriptCore/jit/JIT.h:550:77: error: 'ResultType' has not been declared
> 
> The problem was the following lines in JIT.h:
>         void emitAdd32Constant(unsigned dst, unsigned op, int32_t constant, ResultType opType);
>         void emitSub32Constant(unsigned dst, unsigned op, int32_t constant, ResultType opType);
> 
> Buildfix landed in https://trac.webkit.org/changeset/143154

Ugh, sorry about that.  And thanks for the fix!

[Csaba Osztrogonác]

and it broke QtWebKit build on Mountain Lion:

Undefined symbols for architecture x86_64:
  "JSC::Structure::get(JSC::JSGlobalData&, JSC::PropertyName)", referenced from:
      bool JSC::getStaticFunctionSlot<JSC::InternalFunction>(JSC::ExecState*, JSC::HashTable const*, JSC::JSObject*, JSC::PropertyName, JSC::PropertySlot&) in libJavaScriptCore.a(DateConstructor.o)
      JSC::JSObject::getOwnPropertySlot(JSC::JSCell*, JSC::ExecState*, JSC::PropertyName, JSC::PropertySlot&) in libJavaScriptCore.a(DateInstance.o)
     (maybe you meant: __ZN3JSC9Structure3getERNS_12JSGlobalDataENS_12PropertyNameERjRPNS_6JSCellE)
ld: symbol(s) not found for architecture x86_64
clang: error: linker command failed with exit code 1 (use -v to see invocation)
make[3]: *** [../../bin/jsc] Error 1

Zeno, could you check and fix it, please? (We can't do it without Mac environment.)

[Ádám Kallai]

I fixed it. I added missing header.
http://trac.webkit.org/changeset/143235
http://trac.webkit.org/changeset/143231