Commit b3b9466a authored by Nicolò Ribaudo's avatar Nicolò Ribaudo Committed by V8 LUCI CQ

[class] Improve errors for reinitialized private elements

Previously V8 was reusing the error fur duplicate declarations, using
the private name for class fields or the class name for class methods
as the redeclared identifier.

    class A { constructor(o) { return o } }
    class B extends A { #x }
    class C extends A { #x() {} }
    let D = (0, class extends A { #x() {} });

    new B(new B({})) // Identifier '#x' has already been declared
    new C(new C({})) // Identifier 'C' has already been declared
    new D(new D({})) // Identifier '' has already been declared

This patch changes it to use error messages that better explain what's
happening:

    new B(new B({})) // Cannot initialize #x twice on the same object
    new C(new C({})) // Cannot initialize private methods of
                     // class C twice on the same object
    new D(new D({})) // Cannot initialize private methods of
                     // class anonymous twice on the same object

I initially tried to use the same message for both fields and methods,
but the problem with that is that when initializing fields we only
have access to the field name, while when initializing methods we only
have access to the class name (using the "private brand" symbol).
However, almost all the error messages are different for private fields
and for methods so this shouldn't be a problem.

Bug: v8:12042
Change-Id: Iaa50c16e4fa5c0646ad9ef2aa7e65bb649b3fce2
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3078362Reviewed-by: 's avatarLeszek Swirski <leszeks@chromium.org>
Reviewed-by: 's avatarJoyee Cheung <joyee@igalia.com>
Commit-Queue: Leszek Swirski <leszeks@chromium.org>
Cr-Commit-Position: refs/heads/master@{#76279}
parent f2c4695b
...@@ -172,6 +172,7 @@ Milton Chiang <milton.chiang@mediatek.com> ...@@ -172,6 +172,7 @@ Milton Chiang <milton.chiang@mediatek.com>
Mu Tao <pamilty@gmail.com> Mu Tao <pamilty@gmail.com>
Myeong-bo Shim <m0609.shim@samsung.com> Myeong-bo Shim <m0609.shim@samsung.com>
Nicolas Antonius Ernst Leopold Maria Kaiser <nikai@nikai.net> Nicolas Antonius Ernst Leopold Maria Kaiser <nikai@nikai.net>
Nicolò Ribaudo <nicolo.ribaudo@gmail.com>
Niek van der Maas <mail@niekvandermaas.nl> Niek van der Maas <mail@niekvandermaas.nl>
Niklas Hambüchen <mail@nh2.me> Niklas Hambüchen <mail@nh2.me>
Noj Vek <nojvek@gmail.com> Noj Vek <nojvek@gmail.com>
......
...@@ -439,6 +439,10 @@ namespace internal { ...@@ -439,6 +439,10 @@ namespace internal {
T(InvalidRegExpFlags, "Invalid flags supplied to RegExp constructor '%'") \ T(InvalidRegExpFlags, "Invalid flags supplied to RegExp constructor '%'") \
T(InvalidOrUnexpectedToken, "Invalid or unexpected token") \ T(InvalidOrUnexpectedToken, "Invalid or unexpected token") \
T(InvalidPrivateBrand, "Object must be an instance of class %") \ T(InvalidPrivateBrand, "Object must be an instance of class %") \
T(InvalidPrivateBrandReinitialization, \
"Cannot initialize private methods of class % twice on the same object") \
T(InvalidPrivateFieldReitialization, \
"Cannot initialize % twice on the same object") \
T(InvalidPrivateFieldResolution, \ T(InvalidPrivateFieldResolution, \
"Private field '%' must be declared in an enclosing class") \ "Private field '%' must be declared in an enclosing class") \
T(InvalidPrivateMemberRead, \ T(InvalidPrivateMemberRead, \
......
...@@ -2525,7 +2525,7 @@ void BytecodeGenerator::BuildClassLiteral(ClassLiteral* expr, Register name) { ...@@ -2525,7 +2525,7 @@ void BytecodeGenerator::BuildClassLiteral(ClassLiteral* expr, Register name) {
const AstRawString* class_name = const AstRawString* class_name =
expr->scope()->class_variable() != nullptr expr->scope()->class_variable() != nullptr
? expr->scope()->class_variable()->raw_name() ? expr->scope()->class_variable()->raw_name()
: ast_string_constants()->empty_string(); : ast_string_constants()->anonymous_string();
builder() builder()
->LoadLiteral(class_name) ->LoadLiteral(class_name)
.StoreAccumulatorInRegister(brand) .StoreAccumulatorInRegister(brand)
......
...@@ -49,22 +49,10 @@ MaybeHandle<Object> Runtime::GetObjectProperty( ...@@ -49,22 +49,10 @@ MaybeHandle<Object> Runtime::GetObjectProperty(
if (!it.IsFound() && key->IsSymbol() && if (!it.IsFound() && key->IsSymbol() &&
Symbol::cast(*key).is_private_name()) { Symbol::cast(*key).is_private_name()) {
Handle<Symbol> sym = Handle<Symbol>::cast(key); MessageTemplate message = Symbol::cast(*key).IsPrivateBrand()
Handle<Object> name(sym->description(), isolate); ? MessageTemplate::kInvalidPrivateBrand
DCHECK(name->IsString()); : MessageTemplate::kInvalidPrivateMemberRead;
Handle<String> name_string = Handle<String>::cast(name); THROW_NEW_ERROR(isolate, NewTypeError(message, key, lookup_start_object),
if (sym->IsPrivateBrand()) {
Handle<String> class_name = (name_string->length() == 0)
? isolate->factory()->anonymous_string()
: name_string;
THROW_NEW_ERROR(isolate,
NewTypeError(MessageTemplate::kInvalidPrivateBrand,
class_name, lookup_start_object),
Object);
}
THROW_NEW_ERROR(isolate,
NewTypeError(MessageTemplate::kInvalidPrivateMemberRead,
name_string, lookup_start_object),
Object); Object);
} }
return result; return result;
...@@ -1424,7 +1412,9 @@ RUNTIME_FUNCTION(Runtime_AddPrivateBrand) { ...@@ -1424,7 +1412,9 @@ RUNTIME_FUNCTION(Runtime_AddPrivateBrand) {
if (it.IsFound()) { if (it.IsFound()) {
THROW_NEW_ERROR_RETURN_FAILURE( THROW_NEW_ERROR_RETURN_FAILURE(
isolate, NewTypeError(MessageTemplate::kVarRedeclaration, brand)); isolate,
NewTypeError(MessageTemplate::kInvalidPrivateBrandReinitialization,
brand));
} }
PropertyAttributes attributes = PropertyAttributes attributes =
...@@ -1447,7 +1437,8 @@ RUNTIME_FUNCTION(Runtime_AddPrivateField) { ...@@ -1447,7 +1437,8 @@ RUNTIME_FUNCTION(Runtime_AddPrivateField) {
if (it.IsFound()) { if (it.IsFound()) {
THROW_NEW_ERROR_RETURN_FAILURE( THROW_NEW_ERROR_RETURN_FAILURE(
isolate, NewTypeError(MessageTemplate::kVarRedeclaration, key)); isolate,
NewTypeError(MessageTemplate::kInvalidPrivateFieldReitialization, key));
} }
CHECK(Object::AddDataProperty(&it, value, NONE, Just(kDontThrow), CHECK(Object::AddDataProperty(&it, value, NONE, Just(kDontThrow),
......
...@@ -83,7 +83,7 @@ bytecodes: [ ...@@ -83,7 +83,7 @@ bytecodes: [
B(Mov), R(this), R(0), B(Mov), R(this), R(0),
B(Mov), R(context), R(2), B(Mov), R(context), R(2),
/* 48 E> */ B(CallRuntime), U16(Runtime::kAddPrivateBrand), R(0), U8(3), /* 48 E> */ B(CallRuntime), U16(Runtime::kAddPrivateBrand), R(0), U8(3),
/* 53 S> */ B(Wide), B(LdaSmi), I16(281), /* 53 S> */ B(Wide), B(LdaSmi), I16(283),
B(Star3), B(Star3),
B(LdaConstant), U8(0), B(LdaConstant), U8(0),
B(Star4), B(Star4),
...@@ -114,7 +114,7 @@ bytecodes: [ ...@@ -114,7 +114,7 @@ bytecodes: [
B(Mov), R(this), R(0), B(Mov), R(this), R(0),
B(Mov), R(context), R(2), B(Mov), R(context), R(2),
/* 41 E> */ B(CallRuntime), U16(Runtime::kAddPrivateBrand), R(0), U8(3), /* 41 E> */ B(CallRuntime), U16(Runtime::kAddPrivateBrand), R(0), U8(3),
/* 46 S> */ B(Wide), B(LdaSmi), I16(280), /* 46 S> */ B(Wide), B(LdaSmi), I16(282),
B(Star3), B(Star3),
B(LdaConstant), U8(0), B(LdaConstant), U8(0),
B(Star4), B(Star4),
...@@ -145,7 +145,7 @@ bytecodes: [ ...@@ -145,7 +145,7 @@ bytecodes: [
B(Mov), R(this), R(0), B(Mov), R(this), R(0),
B(Mov), R(context), R(2), B(Mov), R(context), R(2),
/* 48 E> */ B(CallRuntime), U16(Runtime::kAddPrivateBrand), R(0), U8(3), /* 48 E> */ B(CallRuntime), U16(Runtime::kAddPrivateBrand), R(0), U8(3),
/* 53 S> */ B(Wide), B(LdaSmi), I16(281), /* 53 S> */ B(Wide), B(LdaSmi), I16(283),
B(Star3), B(Star3),
B(LdaConstant), U8(0), B(LdaConstant), U8(0),
B(Star4), B(Star4),
...@@ -176,7 +176,7 @@ bytecodes: [ ...@@ -176,7 +176,7 @@ bytecodes: [
B(Mov), R(this), R(0), B(Mov), R(this), R(0),
B(Mov), R(context), R(2), B(Mov), R(context), R(2),
/* 41 E> */ B(CallRuntime), U16(Runtime::kAddPrivateBrand), R(0), U8(3), /* 41 E> */ B(CallRuntime), U16(Runtime::kAddPrivateBrand), R(0), U8(3),
/* 46 S> */ B(Wide), B(LdaSmi), I16(280), /* 46 S> */ B(Wide), B(LdaSmi), I16(282),
B(Star4), B(Star4),
B(LdaConstant), U8(0), B(LdaConstant), U8(0),
B(Star5), B(Star5),
......
...@@ -56,7 +56,7 @@ bytecodes: [ ...@@ -56,7 +56,7 @@ bytecodes: [
B(Mov), R(this), R(0), B(Mov), R(this), R(0),
B(Mov), R(context), R(2), B(Mov), R(context), R(2),
/* 44 E> */ B(CallRuntime), U16(Runtime::kAddPrivateBrand), R(0), U8(3), /* 44 E> */ B(CallRuntime), U16(Runtime::kAddPrivateBrand), R(0), U8(3),
/* 49 S> */ B(Wide), B(LdaSmi), I16(279), /* 49 S> */ B(Wide), B(LdaSmi), I16(281),
B(Star3), B(Star3),
B(LdaConstant), U8(0), B(LdaConstant), U8(0),
B(Star4), B(Star4),
...@@ -88,7 +88,7 @@ bytecodes: [ ...@@ -88,7 +88,7 @@ bytecodes: [
B(Mov), R(this), R(0), B(Mov), R(this), R(0),
B(Mov), R(context), R(2), B(Mov), R(context), R(2),
/* 44 E> */ B(CallRuntime), U16(Runtime::kAddPrivateBrand), R(0), U8(3), /* 44 E> */ B(CallRuntime), U16(Runtime::kAddPrivateBrand), R(0), U8(3),
/* 49 S> */ B(Wide), B(LdaSmi), I16(279), /* 49 S> */ B(Wide), B(LdaSmi), I16(281),
B(Star3), B(Star3),
B(LdaConstant), U8(0), B(LdaConstant), U8(0),
B(Star4), B(Star4),
......
...@@ -24,7 +24,7 @@ bytecodes: [ ...@@ -24,7 +24,7 @@ bytecodes: [
B(TestReferenceEqual), R(this), B(TestReferenceEqual), R(this),
B(Mov), R(this), R(1), B(Mov), R(this), R(1),
B(JumpIfTrue), U8(16), B(JumpIfTrue), U8(16),
B(Wide), B(LdaSmi), I16(277), B(Wide), B(LdaSmi), I16(279),
B(Star2), B(Star2),
B(LdaConstant), U8(0), B(LdaConstant), U8(0),
B(Star3), B(Star3),
...@@ -55,7 +55,7 @@ frame size: 2 ...@@ -55,7 +55,7 @@ frame size: 2
parameter count: 1 parameter count: 1
bytecode array length: 14 bytecode array length: 14
bytecodes: [ bytecodes: [
/* 56 S> */ B(Wide), B(LdaSmi), I16(279), /* 56 S> */ B(Wide), B(LdaSmi), I16(281),
B(Star0), B(Star0),
B(LdaConstant), U8(0), B(LdaConstant), U8(0),
B(Star1), B(Star1),
...@@ -82,7 +82,7 @@ frame size: 2 ...@@ -82,7 +82,7 @@ frame size: 2
parameter count: 1 parameter count: 1
bytecode array length: 14 bytecode array length: 14
bytecodes: [ bytecodes: [
/* 56 S> */ B(Wide), B(LdaSmi), I16(279), /* 56 S> */ B(Wide), B(LdaSmi), I16(281),
B(Star0), B(Star0),
B(LdaConstant), U8(0), B(LdaConstant), U8(0),
B(Star1), B(Star1),
...@@ -121,7 +121,7 @@ bytecodes: [ ...@@ -121,7 +121,7 @@ bytecodes: [
B(TestReferenceEqual), R(this), B(TestReferenceEqual), R(this),
B(Mov), R(this), R(0), B(Mov), R(this), R(0),
B(JumpIfTrue), U8(16), B(JumpIfTrue), U8(16),
B(Wide), B(LdaSmi), I16(277), B(Wide), B(LdaSmi), I16(279),
B(Star2), B(Star2),
B(LdaConstant), U8(0), B(LdaConstant), U8(0),
B(Star3), B(Star3),
...@@ -143,7 +143,7 @@ bytecodes: [ ...@@ -143,7 +143,7 @@ bytecodes: [
B(TestReferenceEqual), R(this), B(TestReferenceEqual), R(this),
B(Mov), R(this), R(1), B(Mov), R(this), R(1),
B(JumpIfTrue), U8(16), B(JumpIfTrue), U8(16),
B(Wide), B(LdaSmi), I16(278), B(Wide), B(LdaSmi), I16(280),
B(Star3), B(Star3),
B(LdaConstant), U8(0), B(LdaConstant), U8(0),
B(Star4), B(Star4),
...@@ -158,7 +158,7 @@ bytecodes: [ ...@@ -158,7 +158,7 @@ bytecodes: [
B(TestReferenceEqual), R(this), B(TestReferenceEqual), R(this),
B(Mov), R(this), R(0), B(Mov), R(this), R(0),
B(JumpIfTrue), U8(16), B(JumpIfTrue), U8(16),
B(Wide), B(LdaSmi), I16(277), B(Wide), B(LdaSmi), I16(279),
B(Star2), B(Star2),
B(LdaConstant), U8(0), B(LdaConstant), U8(0),
B(Star3), B(Star3),
...@@ -188,7 +188,7 @@ frame size: 2 ...@@ -188,7 +188,7 @@ frame size: 2
parameter count: 1 parameter count: 1
bytecode array length: 14 bytecode array length: 14
bytecodes: [ bytecodes: [
/* 60 S> */ B(Wide), B(LdaSmi), I16(281), /* 60 S> */ B(Wide), B(LdaSmi), I16(283),
B(Star0), B(Star0),
B(LdaConstant), U8(0), B(LdaConstant), U8(0),
B(Star1), B(Star1),
...@@ -214,7 +214,7 @@ frame size: 2 ...@@ -214,7 +214,7 @@ frame size: 2
parameter count: 1 parameter count: 1
bytecode array length: 14 bytecode array length: 14
bytecodes: [ bytecodes: [
/* 53 S> */ B(Wide), B(LdaSmi), I16(280), /* 53 S> */ B(Wide), B(LdaSmi), I16(282),
B(Star0), B(Star0),
B(LdaConstant), U8(0), B(LdaConstant), U8(0),
B(Star1), B(Star1),
...@@ -240,7 +240,7 @@ frame size: 2 ...@@ -240,7 +240,7 @@ frame size: 2
parameter count: 1 parameter count: 1
bytecode array length: 14 bytecode array length: 14
bytecodes: [ bytecodes: [
/* 60 S> */ B(Wide), B(LdaSmi), I16(281), /* 60 S> */ B(Wide), B(LdaSmi), I16(283),
B(Star0), B(Star0),
B(LdaConstant), U8(0), B(LdaConstant), U8(0),
B(Star1), B(Star1),
...@@ -266,7 +266,7 @@ frame size: 3 ...@@ -266,7 +266,7 @@ frame size: 3
parameter count: 1 parameter count: 1
bytecode array length: 14 bytecode array length: 14
bytecodes: [ bytecodes: [
/* 46 S> */ B(Wide), B(LdaSmi), I16(280), /* 46 S> */ B(Wide), B(LdaSmi), I16(282),
B(Star1), B(Star1),
B(LdaConstant), U8(0), B(LdaConstant), U8(0),
B(Star2), B(Star2),
......
// Copyright 2019 the V8 project authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
class A { constructor(o) { return o } }
let B = (0, class extends A { #x() { } });
let o = new B({});
new B(o);
*%(basename)s:7: TypeError: Cannot initialize private methods of class anonymous twice on the same object
let B = (0, class extends A { #x() { } });
^
TypeError: Cannot initialize private methods of class anonymous twice on the same object
at new B (*%(basename)s:7:13)
at *%(basename)s:10:1
// Copyright 2019 the V8 project authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
class A { constructor(o) { return o } }
class B extends A { #x() {} }
let o = new B({});
new B(o);
*%(basename)s:7: TypeError: Cannot initialize private methods of class B twice on the same object
class B extends A { #x() {} }
^
TypeError: Cannot initialize private methods of class B twice on the same object
at new B (*%(basename)s:7:1)
at *%(basename)s:10:1
// Copyright 2019 the V8 project authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
class A { constructor(o) { return o } }
class B extends A { #x; }
let o = new B({});
new B(o);
*%(basename)s:7: TypeError: Cannot initialize #x twice on the same object
class B extends A { #x; }
^
TypeError: Cannot initialize #x twice on the same object
at Object.<instance_members_initializer> (*%(basename)s:7:21)
at new B (*%(basename)s:7:1)
at *%(basename)s:10:1
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment