[class] Fix class field name initialization

Previously when class names were computed and set as part of
StoreDataPropertyInLiteral calls, it was observable to static fields
as these static fields are initialized right after the classes were
constructed but before the class names were installed.
This caused the name property to be undefined for this case.

Instead, this patch always forces the creation of a name property on
the class constructor when static class fields are used. This patch
does kill the class boilerplate optimization, but currently all static
class fields are installed using a runtime call to CreateDataProperty
so this isn't any worse when using static class fields.

In the future, this can be optimized away by storing the name on the
boilerplate.

There is spec discussion here:
https://github.com/tc39/proposal-class-fields/issues/85

There isn't a resolution yet, there's still discussion about whether
to have the name be undefined always for static class field
initializers. But, I don't think that's useful as it would always kill
our boilerplate optimization (like this patch does ..., but without the
future optimization potential).

Bug: v8:5367
Change-Id: I14afdf7ece3f2d9fa3c659d2c0bc3806e0b17abb
Reviewed-on: https://chromium-review.googlesource.com/c/1281002
Reviewed-by: Mythri Alle <mythria@chromium.org>
Reviewed-by: Daniel Ehrenberg <littledan@chromium.org>
Reviewed-by: Igor Sheludko <ishell@chromium.org>
Commit-Queue: Sathya Gunasekaran <gsathya@chromium.org>
Cr-Commit-Position: refs/heads/master@{#56686}
This commit is contained in:
Sathya Gunasekaran 2018-10-16 08:49:20 +02:00 committed by Commit Bot
parent 499d7c5a85
commit bc324dbd9b
3 changed files with 54 additions and 5 deletions

View File

@ -1827,7 +1827,7 @@ bool BytecodeGenerator::ShouldOptimizeAsOneShot() const {
return info()->literal()->is_toplevel() || is_toplevel_iife;
}
void BytecodeGenerator::BuildClassLiteral(ClassLiteral* expr) {
void BytecodeGenerator::BuildClassLiteral(ClassLiteral* expr, Register name) {
size_t class_boilerplate_entry =
builder()->AllocateDeferredConstantPoolEntry();
class_literals_.push_back(std::make_pair(expr, class_boilerplate_entry));
@ -1940,6 +1940,24 @@ void BytecodeGenerator::BuildClassLiteral(ClassLiteral* expr) {
}
if (expr->static_fields_initializer() != nullptr) {
// TODO(gsathya): This can be optimized away to be a part of the
// class boilerplate in the future. The name argument can be
// passed to the DefineClass runtime function and have it set
// there.
if (name.is_valid()) {
Register key = register_allocator()->NewRegister();
builder()
->LoadLiteral(ast_string_constants()->name_string())
.StoreAccumulatorInRegister(key);
DataPropertyInLiteralFlags data_property_flags =
DataPropertyInLiteralFlag::kNoFlags;
FeedbackSlot slot =
feedback_spec()->AddStoreDataPropertyInLiteralICSlot();
builder()->LoadAccumulatorWithRegister(name).StoreDataPropertyInLiteral(
class_constructor, key, data_property_flags, feedback_index(slot));
}
RegisterList args = register_allocator()->NewRegisterList(1);
Register initializer =
VisitForRegisterValue(expr->static_fields_initializer());
@ -1961,14 +1979,18 @@ void BytecodeGenerator::BuildClassLiteral(ClassLiteral* expr) {
}
void BytecodeGenerator::VisitClassLiteral(ClassLiteral* expr) {
VisitClassLiteral(expr, Register::invalid_value());
}
void BytecodeGenerator::VisitClassLiteral(ClassLiteral* expr, Register name) {
CurrentScope current_scope(this, expr->scope());
DCHECK_NOT_NULL(expr->scope());
if (expr->scope()->NeedsContext()) {
BuildNewLocalBlockContext(expr->scope());
ContextScope scope(this, expr->scope());
BuildClassLiteral(expr);
BuildClassLiteral(expr, name);
} else {
BuildClassLiteral(expr);
BuildClassLiteral(expr, name);
}
}
@ -2318,7 +2340,20 @@ void BytecodeGenerator::VisitObjectLiteral(ObjectLiteral* expr) {
Register key = register_allocator()->NewRegister();
BuildLoadPropertyKey(property, key);
builder()->SetExpressionPosition(property->value());
Register value = VisitForRegisterValue(property->value());
Register value;
// Static class fields require the name property to be set on
// the class, meaning we can't wait until the
// StoreDataPropertyInLiteral call later to set the name.
if (property->value()->IsClassLiteral() &&
property->value()->AsClassLiteral()->static_fields_initializer() !=
nullptr) {
value = register_allocator()->NewRegister();
VisitClassLiteral(property->value()->AsClassLiteral(), key);
builder()->StoreAccumulatorInRegister(value);
} else {
value = VisitForRegisterValue(property->value());
}
VisitSetHomeObject(value, literal, property);
DataPropertyInLiteralFlags data_property_flags =

View File

@ -193,7 +193,8 @@ class BytecodeGenerator final : public AstVisitor<BytecodeGenerator> {
void VisitArgumentsObject(Variable* variable);
void VisitRestArgumentsArray(Variable* rest);
void VisitCallSuper(Call* call);
void BuildClassLiteral(ClassLiteral* expr);
void BuildClassLiteral(ClassLiteral* expr, Register name);
void VisitClassLiteral(ClassLiteral* expr, Register name);
void VisitNewTargetVariable(Variable* variable);
void VisitThisFunctionVariable(Variable* variable);
void BuildInstanceFieldInitialization(Register constructor,

View File

@ -457,3 +457,16 @@ y()();
assertEquals(1, X.p);
}
{
let p = { z: class { static y = this.name } }
assertEquals(p.z.y, 'z');
let q = { ["z"]: class { static y = this.name } }
assertEquals(q.z.y, 'z');
const C = class {
static x = this.name;
}
assertEquals(C.x, 'C');
}