Bug 110024 - Move all Structure out-of-line inline methods to StructureInlines.h
Summary: Move all Structure out-of-line inline methods to StructureInlines.h
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Filip Pizlo
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-02-16 12:47 PST by Filip Pizlo
Modified: 2013-02-18 09:53 PST (History)
23 users (show)

See Also:

Attachments
patch for landing (24.79 KB, patch)
2013-02-16 12:48 PST, Filip Pizlo 		webkit-ews: commit-queue-
Details | Formatted Diff | Diff
work in progress (73.94 KB, patch)
2013-02-17 01:08 PST, Filip Pizlo 		no flags Details | Formatted Diff | Diff
the patch (77.56 KB, patch)
2013-02-17 01:53 PST, Filip Pizlo 		buildbot: commit-queue-
Details | Formatted Diff | Diff
the patch (77.72 KB, patch)
2013-02-17 14:42 PST, Filip Pizlo 		sam: review+
Details | Formatted Diff | Diff
patch for landing (77.68 KB, patch)
2013-02-17 21:53 PST, Filip Pizlo 		no flags Details | Formatted Diff | Diff
Show Obsolete (4) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Filip Pizlo 2013-02-16 12:47:48 PST 
Already rubber stamped by Mark Hahnenberg.  But I'll put it up to EWS.
Comment 1 Filip Pizlo 2013-02-16 12:48:46 PST 
Created attachment 188723 [details]
patch for landing
Comment 2 WebKit Review Bot 2013-02-16 12:51:28 PST 
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.
Comment 3 Early Warning System Bot 2013-02-16 12:57:40 PST 
Comment on attachment 188723 [details]
patch for landing

Attachment 188723 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/16593722
Comment 4 Early Warning System Bot 2013-02-16 12:57:42 PST 
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
Comment 5 Build Bot 2013-02-16 13:17:46 PST 
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
Comment 6 EFL EWS Bot 2013-02-16 13:39:38 PST 
Comment on attachment 188723 [details]
patch for landing

Attachment 188723 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/16593728
Comment 7 kov's GTK+ EWS bot 2013-02-16 14:39:59 PST 
Comment on attachment 188723 [details]
patch for landing

Attachment 188723 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/16590674
Comment 8 Filip Pizlo 2013-02-17 01:08:10 PST 
Created attachment 188756 [details]
work in progress

It turns out that this is harder than it looked.
Comment 9 Filip Pizlo 2013-02-17 01:53:36 PST 
Created attachment 188759 [details]
the patch
Comment 10 Build Bot 2013-02-17 12:37:45 PST 
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
Comment 11 Filip Pizlo 2013-02-17 14:42:37 PST 
Created attachment 188777 [details]
the patch

Rebased. (A semantically but not textually conflicting change was the reason for the wk2 failure, above.)
Comment 12 WebKit Review Bot 2013-02-17 21:34:57 PST 
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.
Comment 13 Filip Pizlo 2013-02-17 21:53:45 PST 
Created attachment 188795 [details]
patch for landing

I think I figured it out.
Comment 14 WebKit Review Bot 2013-02-17 21:55:57 PST 
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.
Comment 15 Filip Pizlo 2013-02-17 22:31:20 PST 
Landed in http://trac.webkit.org/changeset/143147
Comment 16 Csaba Osztrogonác 2013-02-17 23:42:38 PST 
(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
Comment 17 Filip Pizlo 2013-02-17 23:44:45 PST 
(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!
Comment 18 Csaba Osztrogonác 2013-02-17 23:47:18 PST 
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.)
Comment 19 Ádám Kallai 2013-02-18 09:53:55 PST 
I fixed it. I added missing header.
http://trac.webkit.org/changeset/143235
http://trac.webkit.org/changeset/143231