I've previously thought that "let"/"const" variables declared at the top level of a script are only visible within that script. After closer inspection of the spec, I've found this to be wrong. They should be visible across all scripts in a global lexical environment. I need to fix our implementation to account for this.
Created attachment 260201 [details] WIP I think this is about 98% of the implementation. Still need to dot my i's and cross my t's and write tests.
Created attachment 260229 [details] WIP just about done. I need to write tests still, and solve one more problem: <script> let b = false; function foo() { if (b) return x; } foo(); // causes "x" to get linked as GlobalProperty. </script> <script> // foo(); // You can also consider the problem here, if we call "foo" before instantiating "x", how do we implement the TDZ. let x = 50; foo(); // Throws because "x" isn't a global property! </script> I don't have a great solution for this yet.
Attachment 260229 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/bytecode/EvalCodeCache.h:36: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalLexicalEnvironment.h:52: The parameter name "object" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/JavaScriptCore/runtime/JSGlobalLexicalEnvironment.h:52: The parameter name "exec" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/JavaScriptCore/runtime/JSGlobalLexicalEnvironment.h:52: The parameter name "propertyName" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/JavaScriptCore/runtime/JSGlobalLexicalEnvironment.h:52: The parameter name "slot" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/JavaScriptCore/runtime/JSGlobalLexicalEnvironment.h:53: The parameter name "cell" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/JavaScriptCore/runtime/JSGlobalLexicalEnvironment.h:53: The parameter name "exec" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/JavaScriptCore/runtime/JSGlobalLexicalEnvironment.h:53: The parameter name "propertyName" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/JavaScriptCore/runtime/JSGlobalLexicalEnvironment.h:53: The parameter name "value" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/JavaScriptCore/runtime/JSGlobalLexicalEnvironment.h:53: The parameter name "slot" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/JavaScriptCore/runtime/Executable.cpp:493: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] ERROR: Source/JavaScriptCore/runtime/Executable.cpp:498: This { should be at the end of the previous line [whitespace/braces] [4] ERROR: Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp:75: One space before end of line comments [whitespace/comments] [5] ERROR: Source/JavaScriptCore/jit/JITPropertyAccess.cpp:882: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/JavaScriptCore/jit/JITPropertyAccess.cpp:883: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Total errors found: 15 in 38 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 260388 [details] WIP This is almost done. Still need to write 32-bit portion, still need to write tests.
Created attachment 260403 [details] patch some tests. Still no 32-bit.
Attachment 260403 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/runtime/JSGlobalLexicalEnvironment.h:52: The parameter name "object" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/JavaScriptCore/runtime/JSGlobalLexicalEnvironment.h:52: The parameter name "exec" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/JavaScriptCore/runtime/JSGlobalLexicalEnvironment.h:52: The parameter name "propertyName" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/JavaScriptCore/runtime/JSGlobalLexicalEnvironment.h:52: The parameter name "slot" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/JavaScriptCore/runtime/JSGlobalLexicalEnvironment.h:53: The parameter name "cell" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/JavaScriptCore/runtime/JSGlobalLexicalEnvironment.h:53: The parameter name "exec" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/JavaScriptCore/runtime/JSGlobalLexicalEnvironment.h:53: The parameter name "propertyName" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/JavaScriptCore/runtime/JSGlobalLexicalEnvironment.h:53: The parameter name "value" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/JavaScriptCore/runtime/JSGlobalLexicalEnvironment.h:53: The parameter name "slot" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/JavaScriptCore/runtime/CommonSlowPaths.cpp:667: Should have a space between // and comment [whitespace/comments] [4] ERROR: Source/JavaScriptCore/runtime/CommonSlowPaths.cpp:674: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/JavaScriptCore/runtime/CommonSlowPaths.cpp:675: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/JavaScriptCore/runtime/CommonSlowPaths.cpp:676: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/JavaScriptCore/runtime/CommonSlowPaths.cpp:677: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/JavaScriptCore/runtime/CommonSlowPaths.cpp:678: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/JavaScriptCore/runtime/CommonSlowPaths.cpp:681: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/JavaScriptCore/runtime/CommonSlowPaths.cpp:682: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/JavaScriptCore/runtime/CommonSlowPaths.cpp:683: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/JavaScriptCore/runtime/CommonSlowPaths.cpp:684: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/JavaScriptCore/runtime/CommonSlowPaths.cpp:686: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/JavaScriptCore/runtime/Executable.cpp:555: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] ERROR: Source/JavaScriptCore/runtime/Executable.cpp:560: This { should be at the end of the previous line [whitespace/braces] [4] ERROR: Source/JavaScriptCore/CMakeLists.txt:529: Alphabetical sorting problem. "runtime/JSGlobalLexicalEnvironment.cpp" should be before "runtime/JSGlobalObject.cpp". [list/order] [5] ERROR: Source/JavaScriptCore/llint/LLIntData.cpp:156: Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons. [readability/comparison_to_zero] [5] ERROR: Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp:75: One space before end of line comments [whitespace/comments] [5] ERROR: Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h:2366: Should have a space between // and comment [whitespace/comments] [4] ERROR: Source/JavaScriptCore/jit/JITPropertyAccess.cpp:37: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/JavaScriptCore/jit/JITPropertyAccess.cpp:928: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/JavaScriptCore/jit/JITPropertyAccess.cpp:929: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/JavaScriptCore/runtime/GetPutInfo.h:66: One space before end of line comments [whitespace/comments] [5] Total errors found: 30 in 63 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 260403 [details] patch Attachment 260403 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.webkit.org/results/131762 New failing tests: js/dom/const.html
Created attachment 260404 [details] Archive of layout-test-results from ews104 for mac-mavericks-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews104 Port: mac-mavericks-wk2 Platform: Mac OS X 10.9.5
Comment on attachment 260403 [details] patch Attachment 260403 [details] did not pass mac-ews (mac): Output: http://webkit-queues.webkit.org/results/131788 New failing tests: js/dom/const.html
Created attachment 260405 [details] Archive of layout-test-results from ews101 for mac-mavericks The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews101 Port: mac-mavericks Platform: Mac OS X 10.9.5
Created attachment 260429 [details] patch still needs a changelog,
Attachment 260429 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/llint/LLIntData.cpp:156: Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons. [readability/comparison_to_zero] [5] ERROR: Source/JavaScriptCore/jit/JITPropertyAccess.cpp:900: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/JavaScriptCore/jit/JITPropertyAccess.cpp:901: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/JavaScriptCore/runtime/GetPutInfo.h:66: One space before end of line comments [whitespace/comments] [5] Total errors found: 4 in 80 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 260429 [details] patch Attachment 260429 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.webkit.org/results/133954 New failing tests: js/dom/const.html
Created attachment 260433 [details] Archive of layout-test-results from ews106 for mac-mavericks-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews106 Port: mac-mavericks-wk2 Platform: Mac OS X 10.9.5
Comment on attachment 260429 [details] patch Attachment 260429 [details] did not pass mac-ews (mac): Output: http://webkit-queues.webkit.org/results/133960 New failing tests: js/dom/const.html
Created attachment 260435 [details] Archive of layout-test-results from ews101 for mac-mavericks The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews101 Port: mac-mavericks Platform: Mac OS X 10.9.5
Created attachment 260438 [details] patch
Attachment 260438 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/llint/LLIntData.cpp:156: Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons. [readability/comparison_to_zero] [5] ERROR: Source/JavaScriptCore/jit/JITPropertyAccess.cpp:900: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/JavaScriptCore/jit/JITPropertyAccess.cpp:901: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/JavaScriptCore/runtime/GetPutInfo.h:66: One space before end of line comments [whitespace/comments] [5] Total errors found: 4 in 82 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 260440 [details] patch
Attachment 260440 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/llint/LLIntData.cpp:156: Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons. [readability/comparison_to_zero] [5] ERROR: Source/JavaScriptCore/jit/JITPropertyAccess.cpp:900: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/JavaScriptCore/jit/JITPropertyAccess.cpp:901: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/JavaScriptCore/runtime/GetPutInfo.h:66: One space before end of line comments [whitespace/comments] [5] Total errors found: 4 in 81 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 260447 [details] patch
Attachment 260447 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/llint/LLIntData.cpp:156: Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons. [readability/comparison_to_zero] [5] ERROR: Source/JavaScriptCore/jit/JITPropertyAccess.cpp:900: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/JavaScriptCore/jit/JITPropertyAccess.cpp:901: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/JavaScriptCore/runtime/GetPutInfo.h:66: One space before end of line comments [whitespace/comments] [5] Total errors found: 4 in 81 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 260440 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=260440&action=review r=me with comment > Source/JavaScriptCore/dfg/DFGAbstractValue.h:78 > + > + void makeHeapTopOrEmpty() > + { > + makeTop(SpecHeapTop | SpecEmpty); > + } Hmmm, I think that this is just called BytecodeTop. We should clarify in SpeculatedType.h that "bytecode top" just means "heap top plus empty". And you should remove this method.
(In reply to comment #23) > Comment on attachment 260440 [details] > patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=260440&action=review > > r=me with comment > > > Source/JavaScriptCore/dfg/DFGAbstractValue.h:78 > > + > > + void makeHeapTopOrEmpty() > > + { > > + makeTop(SpecHeapTop | SpecEmpty); > > + } > > Hmmm, I think that this is just called BytecodeTop. We should clarify in > SpeculatedType.h that "bytecode top" just means "heap top plus empty". And > you should remove this method. Okay. Thanks for the review. I've added a comment.
landed in: http://trac.webkit.org/changeset/189279
This regressed SunSpider by close to 1%.