Reviewers: arv,
Message:
WDYT Erik? Also is the "scope_uses_super_property" thing actually needed?
I
could remove that too.
Regarding MayUseThis: I initially wanted to walk the scope chain until I
got to
one with has_this_declaration() and then grab the receiver() but scopes
deserialized from ScopeInfo don't get receiver_ set. So, LookupThis()
instead.
I guess I could instead ensure that receiver_ gets set and replace
LookupThis()
with this_declaration_scope(); wdyt?
Description:
Remove Scope::scope_uses_this_ flag
[email protected]
LOG=N
BUG=
Please review this at https://codereview.chromium.org/1129823002/
Base URL: https://chromium.googlesource.com/v8/v8@master
Affected files (+12, -36 lines):
M src/compiler.cc
M src/compiler/ast-graph-builder.cc
M src/preparser.h
M src/scopes.h
M src/scopes.cc
M test/cctest/test-parsing.cc
Index: src/compiler.cc
diff --git a/src/compiler.cc b/src/compiler.cc
index
2ad1ff447e26d51a7b4750ab979e8a9b96d0a1bb..262817c912ba9750b6acb6db8cb9cf7cdb3d175a
100644
--- a/src/compiler.cc
+++ b/src/compiler.cc
@@ -215,8 +215,7 @@ bool CompilationInfo::is_simple_parameter_list() {
bool CompilationInfo::MayUseThis() const {
- return scope()->uses_this() || scope()->inner_uses_this() ||
- scope()->calls_sloppy_eval();
+ return scope()->has_this_declaration() && scope()->receiver()->is_used();
}
Index: src/compiler/ast-graph-builder.cc
diff --git a/src/compiler/ast-graph-builder.cc
b/src/compiler/ast-graph-builder.cc
index
2ef8d6ffcb6914a7e6f01f811fe0eb1bbcdf0061..26983d6e10fffc9f6c985bd9f100ce11ccfc123d
100644
--- a/src/compiler/ast-graph-builder.cc
+++ b/src/compiler/ast-graph-builder.cc
@@ -2661,9 +2661,7 @@ Node*
AstGraphBuilder::BuildPatchReceiverToGlobalProxy(Node* receiver) {
// object). Otherwise there is nothing left to do here.
if (is_strict(language_mode()) || info()->is_native()) return receiver;
- // There is no need to perform patching if the receiver is never used.
Note
- // that scope predicates are purely syntactical, a call to eval might
still
- // inspect the receiver value.
+ // There is no need to perform patching if the receiver will never be
used.
if (!info()->MayUseThis()) return receiver;
IfBuilder receiver_check(this);
Index: src/preparser.h
diff --git a/src/preparser.h b/src/preparser.h
index
8184bd8d5f5af2bc93a872c80c71ab3b7cce8001..543f82ee56a281dfc5984a7aa9a9be91319cf659
100644
--- a/src/preparser.h
+++ b/src/preparser.h
@@ -2215,7 +2215,6 @@
ParserBase<Traits>::ParsePrimaryExpression(ExpressionClassifier* classifier,
break;
}
}
- scope_->RecordThisUsage();
result = this->ThisExpression(scope_, factory(), beg_pos);
break;
}
@@ -3241,7 +3240,6 @@
ParserBase<Traits>::ParseStrongInitializationExpression(
Consume(Token::THIS);
int pos = position();
function_state_->set_this_location(scanner()->location());
- scope_->RecordThisUsage();
ExpressionT this_expr = this->ThisExpression(scope_, factory(), pos);
ExpressionT left = this->EmptyExpression();
Index: src/scopes.cc
diff --git a/src/scopes.cc b/src/scopes.cc
index
32bd4bfc41261485b9cbdeb3eb004e157b056c56..8d23180f9e407bb43cb4a50599e6b130a5126c8b
100644
--- a/src/scopes.cc
+++ b/src/scopes.cc
@@ -165,7 +165,6 @@ void Scope::SetDefaults(ScopeType scope_type, Scope*
outer_scope,
scope_calls_eval_ = false;
scope_uses_arguments_ = false;
scope_uses_super_property_ = false;
- scope_uses_this_ = false;
asm_module_ = false;
asm_function_ = outer_scope != NULL && outer_scope->asm_module_;
// Inherit the language mode from the parent scope.
@@ -173,7 +172,6 @@ void Scope::SetDefaults(ScopeType scope_type, Scope*
outer_scope,
outer_scope_calls_sloppy_eval_ = false;
inner_scope_calls_eval_ = false;
inner_scope_uses_arguments_ = false;
- inner_scope_uses_this_ = false;
inner_scope_uses_super_property_ = false;
force_eager_compilation_ = false;
force_context_allocation_ = (outer_scope != NULL && !is_function_scope())
@@ -371,7 +369,6 @@ Scope* Scope::FinalizeBlockScope() {
// Propagate usage flags to outer scope.
if (uses_arguments()) outer_scope_->RecordArgumentsUsage();
if (uses_super_property()) outer_scope_->RecordSuperPropertyUsage();
- if (uses_this()) outer_scope_->RecordThisUsage();
if (scope_calls_eval_) outer_scope_->RecordEvalCall();
return NULL;
@@ -921,13 +918,11 @@ void Scope::Print(int n) {
if (scope_uses_arguments_) Indent(n1, "// scope uses 'arguments'\n");
if (scope_uses_super_property_)
Indent(n1, "// scope uses 'super' property\n");
- if (scope_uses_this_) Indent(n1, "// scope uses 'this'\n");
if (inner_scope_uses_arguments_) {
Indent(n1, "// inner scope uses 'arguments'\n");
}
if (inner_scope_uses_super_property_)
Indent(n1, "// inner scope uses 'super' property\n");
- if (inner_scope_uses_this_) Indent(n1, "// inner scope uses 'this'\n");
if (outer_scope_calls_sloppy_eval_) {
Indent(n1, "// outer scope calls 'eval' in sloppy context\n");
}
@@ -1283,9 +1278,6 @@ void Scope::PropagateScopeInfo(bool
outer_scope_calls_sloppy_eval ) {
inner->inner_scope_uses_super_property_) {
inner_scope_uses_super_property_ = true;
}
- if (inner->scope_uses_this_ || inner->inner_scope_uses_this_) {
- inner_scope_uses_this_ = true;
- }
}
if (inner->force_eager_compilation_) {
force_eager_compilation_ = true;
Index: src/scopes.h
diff --git a/src/scopes.h b/src/scopes.h
index
92f5c4cf72e73aff6bbdf5e09914de835d535f45..f3c64f9eadf73c7c4f9c1671779c4f55926697f9
100644
--- a/src/scopes.h
+++ b/src/scopes.h
@@ -218,9 +218,6 @@ class Scope: public ZoneObject {
// Inform the scope that the corresponding code uses "super".
void RecordSuperPropertyUsage() { scope_uses_super_property_ = true; }
- // Inform the scope that the corresponding code uses "this".
- void RecordThisUsage() { scope_uses_this_ = true; }
-
// Set the language mode flag (unless disabled by a global flag).
void SetLanguageMode(LanguageMode language_mode) {
language_mode_ = language_mode;
@@ -324,10 +321,6 @@ class Scope: public ZoneObject {
bool inner_uses_super_property() const {
return inner_scope_uses_super_property_;
}
- // Does this scope access "this".
- bool uses_this() const { return scope_uses_this_; }
- // Does any inner scope access "this".
- bool inner_uses_this() const { return inner_scope_uses_this_; }
const Scope* NearestOuterEvalScope() const {
if (is_eval_scope()) return this;
@@ -592,8 +585,6 @@ class Scope: public ZoneObject {
bool scope_uses_arguments_;
// This scope uses "super" property ('super.foo').
bool scope_uses_super_property_;
- // This scope uses "this".
- bool scope_uses_this_;
// This scope contains an "use asm" annotation.
bool asm_module_;
// This scope's outer context is an asm module.
@@ -609,7 +600,6 @@ class Scope: public ZoneObject {
bool inner_scope_calls_eval_;
bool inner_scope_uses_arguments_;
bool inner_scope_uses_super_property_;
- bool inner_scope_uses_this_;
bool force_eager_compilation_;
bool force_context_allocation_;
Index: test/cctest/test-parsing.cc
diff --git a/test/cctest/test-parsing.cc b/test/cctest/test-parsing.cc
index
c51aa95b14a93b618090836e497b75d763978337..f0abbc958f9a241f2270bc85864db9561817c34b
100644
--- a/test/cctest/test-parsing.cc
+++ b/test/cctest/test-parsing.cc
@@ -962,7 +962,6 @@ TEST(ScopeUsesArgumentsSuperThis) {
THIS = 1 << 2,
INNER_ARGUMENTS = 1 << 3,
INNER_SUPER_PROPERTY = 1 << 4,
- INNER_THIS = 1 << 5
};
static const struct {
@@ -977,7 +976,7 @@ TEST(ScopeUsesArgumentsSuperThis) {
{"return this + arguments[0]", ARGUMENTS | THIS},
{"return this + arguments[0] + super.x",
ARGUMENTS | SUPER_PROPERTY | THIS},
- {"return x => this + x", INNER_THIS},
+ {"return x => this + x", THIS},
{"return x => super.f() + x", INNER_SUPER_PROPERTY},
{"this.foo = 42;", THIS},
{"this.foo();", THIS},
@@ -991,9 +990,7 @@ TEST(ScopeUsesArgumentsSuperThis) {
{"while (true) { while (true) { while (true) return this } }", THIS},
{"while (true) { while (true) { while (true) return super.f() } }",
SUPER_PROPERTY},
- {"if (1) { return () => { while (true) new this() } }", INNER_THIS},
- // Note that propagation of the inner_uses_this() value does not
- // cross boundaries of normal functions onto parent scopes.
+ {"if (1) { return () => { while (true) new this() } }", THIS},
{"return function (x) { return this + x }", NONE},
{"return { m(x) { return super.m() + x } }", NONE},
{"var x = function () { this.foo = 42 };", NONE},
@@ -1004,10 +1001,10 @@ TEST(ScopeUsesArgumentsSuperThis) {
{"return { m(x) { return () => super.m() } }", NONE},
// Flags must be correctly set when using block scoping.
{"\"use strict\"; while (true) { let x; this, arguments; }",
- INNER_ARGUMENTS | INNER_THIS},
+ INNER_ARGUMENTS | THIS},
{"\"use strict\"; while (true) { let x; this, super.f(), arguments; }",
- INNER_ARGUMENTS | INNER_SUPER_PROPERTY | INNER_THIS},
- {"\"use strict\"; if (foo()) { let x; this.f() }", INNER_THIS},
+ INNER_ARGUMENTS | INNER_SUPER_PROPERTY | THIS},
+ {"\"use strict\"; if (foo()) { let x; this.f() }", THIS},
{"\"use strict\"; if (foo()) { let x; super.f() }",
INNER_SUPER_PROPERTY},
{"\"use strict\"; if (1) {"
@@ -1071,13 +1068,15 @@ TEST(ScopeUsesArgumentsSuperThis) {
scope->uses_arguments());
CHECK_EQ((source_data[i].expected & SUPER_PROPERTY) != 0,
scope->uses_super_property());
- CHECK_EQ((source_data[i].expected & THIS) != 0, scope->uses_this());
+ if ((source_data[i].expected & THIS) != 0) {
+ // Currently the is_used() flag is conservative; all variables in a
+ // script scope are marked as used.
+ CHECK(scope->LookupThis()->is_used());
+ }
CHECK_EQ((source_data[i].expected & INNER_ARGUMENTS) != 0,
scope->inner_uses_arguments());
CHECK_EQ((source_data[i].expected & INNER_SUPER_PROPERTY) != 0,
scope->inner_uses_super_property());
- CHECK_EQ((source_data[i].expected & INNER_THIS) != 0,
- scope->inner_uses_this());
}
}
}
--
--
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/d/optout.
