Revision: 19505
Author: [email protected]
Date: Thu Feb 20 11:35:37 2014 UTC
Log: Re-enable Parser::symbol_cache_ (after a long time!)
The Parser never used the symbol stream produced by the PreParser for
anything
useful, due to a bug introduced 3.5 years ago by
https://codereview.chromium.org/3356010/diff/7001/src/parser.cc.
The bug is that calling Initialize on symbol_cache_ doesn't change its
length. So the length remains 0, and the "if" in Parser::LookupSymbol is
always
true, and Parser::LookupCachedSymbol is never called and symbol_cache_ never
filled.
This bug also masked a bug that the symbol stream produced by PreParser
doesn't
match what Parser wants to consume. The repro case is the following:
var myo = {if: 4}; print(myo.if);
PreParser doesn't log a symbol for the first "if", but in the corresponding
place, Parser consumes one symbol from the symbol stream. Since the consumed
symbols were never really used, this mismatch went unnoticed.
This CL also fixes that bug.
BUG=
[email protected]
Review URL: https://codereview.chromium.org/172753002
http://code.google.com/p/v8/source/detail?r=19505
Modified:
/branches/bleeding_edge/src/parser.cc
/branches/bleeding_edge/src/preparser.cc
/branches/bleeding_edge/test/cctest/cctest.h
/branches/bleeding_edge/test/cctest/test-parsing.cc
=======================================
--- /branches/bleeding_edge/src/parser.cc Wed Feb 19 14:50:33 2014 UTC
+++ /branches/bleeding_edge/src/parser.cc Thu Feb 20 11:35:37 2014 UTC
@@ -207,12 +207,12 @@
Handle<String> Parser::LookupSymbol(int symbol_id) {
- // Length of symbol cache is the number of identified symbols.
- // If we are larger than that, or negative, it's not a cached symbol.
- // This might also happen if there is no preparser symbol data, even
- // if there is some preparser data.
- if (static_cast<unsigned>(symbol_id)
- >= static_cast<unsigned>(symbol_cache_.length())) {
+ // If there is no preparser symbol data, a negative number will be
passed. In
+ // that case, we'll just read the literal from Scanner. This also guards
+ // against corrupt preparse data where the symbol id is larger than the
symbol
+ // count.
+ if (symbol_id < 0 ||
+ (pre_parse_data_ && symbol_id >= pre_parse_data_->symbol_count())) {
if (scanner()->is_literal_ascii()) {
return isolate()->factory()->InternalizeOneByteString(
Vector<const uint8_t>::cast(scanner()->literal_ascii_string()));
=======================================
--- /branches/bleeding_edge/src/preparser.cc Wed Feb 19 14:50:33 2014 UTC
+++ /branches/bleeding_edge/src/preparser.cc Thu Feb 20 11:35:37 2014 UTC
@@ -1184,6 +1184,7 @@
if (Token::IsKeyword(next)) {
Consume(next);
checker.CheckProperty(next, kValueProperty, CHECK_OK);
+ LogSymbol();
} else {
// Unexpected token.
*ok = false;
=======================================
--- /branches/bleeding_edge/test/cctest/cctest.h Tue Feb 4 12:23:30 2014
UTC
+++ /branches/bleeding_edge/test/cctest/cctest.h Thu Feb 20 11:35:37 2014
UTC
@@ -313,6 +313,18 @@
return v8::Script::Compile(
v8::String::NewFromUtf8(v8::Isolate::GetCurrent(), source))->Run();
}
+
+
+static inline v8::Local<v8::Value> PreCompileCompileRun(const char*
source) {
+ v8::Local<v8::String> script_source =
+ v8::String::NewFromUtf8(v8::Isolate::GetCurrent(), source);
+ v8::ScriptData* preparse = v8::ScriptData::PreCompile(script_source);
+ v8::Local<v8::Script> script =
+ v8::Script::Compile(script_source, NULL, preparse);
+ v8::Local<v8::Value> result = script->Run();
+ delete preparse;
+ return result;
+}
// Helper function that compiles and runs the source with given origin.
=======================================
--- /branches/bleeding_edge/test/cctest/test-parsing.cc Mon Feb 17 15:40:51
2014 UTC
+++ /branches/bleeding_edge/test/cctest/test-parsing.cc Thu Feb 20 11:35:37
2014 UTC
@@ -322,6 +322,43 @@
CHECK(data.has_error());
}
}
+
+
+TEST(PreparsingObjectLiterals) {
+ // Regression test for a bug where the symbol stream produced by
PreParser
+ // didn't match what Parser wanted to consume.
+ v8::Isolate* isolate = CcTest::isolate();
+ v8::HandleScope handles(isolate);
+ v8::Local<v8::Context> context = v8::Context::New(isolate);
+ v8::Context::Scope context_scope(context);
+ int marker;
+ CcTest::i_isolate()->stack_guard()->SetStackLimit(
+ reinterpret_cast<uintptr_t>(&marker) - 128 * 1024);
+
+ {
+ const char* source = "var myo = {if: \"foo\"}; myo.if;";
+ v8::Local<v8::Value> result = PreCompileCompileRun(source);
+ CHECK(result->IsString());
+ v8::String::Utf8Value utf8(result);
+ CHECK_EQ("foo", *utf8);
+ }
+
+ {
+ const char* source = "var myo = {\"bar\": \"foo\"}; myo[\"bar\"];";
+ v8::Local<v8::Value> result = PreCompileCompileRun(source);
+ CHECK(result->IsString());
+ v8::String::Utf8Value utf8(result);
+ CHECK_EQ("foo", *utf8);
+ }
+
+ {
+ const char* source = "var myo = {1: \"foo\"}; myo[1];";
+ v8::Local<v8::Value> result = PreCompileCompileRun(source);
+ CHECK(result->IsString());
+ v8::String::Utf8Value utf8(result);
+ CHECK_EQ("foo", *utf8);
+ }
+}
TEST(RegressChromium62639) {
--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/groups/opt_out.
