Skip to content
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
deps: V8: cherry-pick ad21d212fc14
Original commit message:

    Preserve "proper method names" as-is in error.stack.

    This changes the logic for generating method names in `error.stack` to
    prepend an inferred type name only when the function name is a valid
    ECMAScript identifiers and does not equal the inferred type name, to

    (1) give developers more control over the exact name shown in
        `error.stack`, as well as
    (2) avoid confusion in the presence of renaming of local variables.

    Previously we'd leave the function name as-is if it was prefixed by the
    inferred type name, but that condition is unnecessarily strict, and led
    to a bunch of inconsistencies around special names like
    `<instance_member_initializer>` where this dynamic approached often
    prefixed it with the correct type name, but also sometimes got it wrong
    and prepended `Object.`, which is very unfortunate and misleading.
    Specifically for these special names, we'll add logic later in the
    parser to infer a useful (complete) name.

    The design doc (https://bit.ly/devtools-method-names-in-stack-traces)
    contains more background and examples of why we do this change.

    Doc: https://bit.ly/devtools-method-names-in-stack-traces
    Fixed: chromium:1294619
    Bug: chromium:1283435
    Change-Id: Ib8b528ba25255dcd07e9d11044c562c11d699bcb
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3565724
    Reviewed-by: Yang Guo <yangguo@chromium.org>
    Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
    Cr-Commit-Position: refs/heads/main@{#79748}

Refs: v8/v8@ad21d21
  • Loading branch information
targos committed Apr 12, 2022
commit 5a7a632a2a5e629314a787b89a22057cc0cb84b0
2 changes: 1 addition & 1 deletion common.gypi
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@

# Reset this number to 0 on major V8 upgrades.
# Increment by one for each non-official patch applied to deps/v8.
'v8_embedder_string': '-node.11',
'v8_embedder_string': '-node.12',

##### V8 defaults for Node.js #####

Expand Down
25 changes: 1 addition & 24 deletions deps/v8/src/api/api.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2537,29 +2537,6 @@ MaybeLocal<Module> ScriptCompiler::CompileModule(
return ToApiHandle<Module>(i_isolate->factory()->NewSourceTextModule(shared));
}

namespace {
bool IsIdentifier(i::Isolate* isolate, i::Handle<i::String> string) {
string = i::String::Flatten(isolate, string);
const int length = string->length();
if (length == 0) return false;
if (!i::IsIdentifierStart(string->Get(0))) return false;
i::DisallowGarbageCollection no_gc;
i::String::FlatContent flat = string->GetFlatContent(no_gc);
if (flat.IsOneByte()) {
auto vector = flat.ToOneByteVector();
for (int i = 1; i < length; i++) {
if (!i::IsIdentifierPart(vector[i])) return false;
}
} else {
auto vector = flat.ToUC16Vector();
for (int i = 1; i < length; i++) {
if (!i::IsIdentifierPart(vector[i])) return false;
}
}
return true;
}
} // namespace

// static
V8_WARN_UNUSED_RESULT MaybeLocal<Function> ScriptCompiler::CompileFunction(
Local<Context> context, Source* source, size_t arguments_count,
Expand Down Expand Up @@ -2608,7 +2585,7 @@ MaybeLocal<Function> ScriptCompiler::CompileFunctionInternal(
isolate->factory()->NewFixedArray(static_cast<int>(arguments_count));
for (int i = 0; i < static_cast<int>(arguments_count); i++) {
i::Handle<i::String> argument = Utils::OpenHandle(*arguments[i]);
if (!IsIdentifier(isolate, argument)) return Local<Function>();
if (!i::String::IsIdentifier(isolate, argument)) return Local<Function>();
arguments_list->set(i, *argument);
}

Expand Down
11 changes: 2 additions & 9 deletions deps/v8/src/objects/call-site-info.cc
Original file line number Diff line number Diff line change
Expand Up @@ -629,12 +629,6 @@ void AppendFileLocation(Isolate* isolate, Handle<CallSiteInfo> frame,
}
}

int StringIndexOf(Isolate* isolate, Handle<String> subject,
Handle<String> pattern) {
if (pattern->length() > subject->length()) return -1;
return String::IndexOf(isolate, subject, pattern, 0);
}

// Returns true iff
// 1. the subject ends with '.' + pattern, or
// 2. subject == pattern.
Expand Down Expand Up @@ -676,9 +670,8 @@ void AppendMethodCall(Isolate* isolate, Handle<CallSiteInfo> frame,
Handle<String> function_string = Handle<String>::cast(function_name);
if (IsNonEmptyString(type_name)) {
Handle<String> type_string = Handle<String>::cast(type_name);
bool starts_with_type_name =
(StringIndexOf(isolate, function_string, type_string) == 0);
if (!starts_with_type_name) {
if (String::IsIdentifier(isolate, function_string) &&
!String::Equals(isolate, function_string, type_string)) {
builder->AppendString(type_string);
builder->AppendCharacter('.');
}
Expand Down
29 changes: 29 additions & 0 deletions deps/v8/src/objects/string.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1558,6 +1558,35 @@ bool String::HasOneBytePrefix(base::Vector<const char> str) {

namespace {

template <typename Char>
bool IsIdentifierVector(const base::Vector<Char>& vec) {
if (vec.empty()) {
return false;
}
if (!IsIdentifierStart(vec[0])) {
return false;
}
for (size_t i = 1; i < vec.size(); ++i) {
if (!IsIdentifierPart(vec[i])) {
return false;
}
}
return true;
}

} // namespace

// static
bool String::IsIdentifier(Isolate* isolate, Handle<String> str) {
str = String::Flatten(isolate, str);
DisallowGarbageCollection no_gc;
String::FlatContent flat = str->GetFlatContent(no_gc);
return flat.IsOneByte() ? IsIdentifierVector(flat.ToOneByteVector())
: IsIdentifierVector(flat.ToUC16Vector());
}

namespace {

template <typename Char>
uint32_t HashString(String string, size_t start, int length, uint64_t seed,
PtrComprCageBase cage_base,
Expand Down
3 changes: 3 additions & 0 deletions deps/v8/src/objects/string.h
Original file line number Diff line number Diff line change
Expand Up @@ -391,6 +391,9 @@ class String : public TorqueGeneratedString<String, Name> {
V8_EXPORT_PRIVATE bool HasOneBytePrefix(base::Vector<const char> str);
V8_EXPORT_PRIVATE inline bool IsOneByteEqualTo(base::Vector<const char> str);

// Returns true if the |str| is a valid ECMAScript identifier.
static bool IsIdentifier(Isolate* isolate, Handle<String> str);

// Return a UTF8 representation of the string. The string is null
// terminated but may optionally contain nulls. Length is returned
// in length_output if length_output is not a null pointer The string
Expand Down
2 changes: 0 additions & 2 deletions deps/v8/test/message/fail/class-fields-static-throw.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
// Copyright 2018 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.
//
// TODO(gsathya): Remove 'Function' from stack trace.

class X {
static x = foo();
Expand Down
4 changes: 2 additions & 2 deletions deps/v8/test/message/fail/class-fields-static-throw.out
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
*%(basename)s:8: ReferenceError: foo is not defined
*%(basename)s:6: ReferenceError: foo is not defined
static x = foo();
^
ReferenceError: foo is not defined
at Function.<static_initializer> (*%(basename)s:8:14)
at <static_initializer> (*%(basename)s:6:14)
at *%(basename)s:1:1
4 changes: 2 additions & 2 deletions deps/v8/test/message/fail/class-fields-throw.out
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,6 @@
x = foo();
^
ReferenceError: foo is not defined
at X.<instance_members_initializer> (*%(basename)s:6:7)
at <instance_members_initializer> (*%(basename)s:6:7)
at new X (*%(basename)s:5:1)
at *%(basename)s:9:1
at *%(basename)s:9:1
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,6 @@
class B extends A { #x; }
^
TypeError: Cannot initialize #x twice on the same object
at Object.<instance_members_initializer> (*%(basename)s:7:21)
at <instance_members_initializer> (*%(basename)s:7:21)
at new B (*%(basename)s:7:1)
at *%(basename)s:10:1
12 changes: 12 additions & 0 deletions deps/v8/test/message/fail/stack-trace-method-name-configured.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
// Copyright 2022 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.

// Check that the user configured method name ('SomeClass.someMethod') is used
// as-is in error.stack output without prepending the inferred type name ('A').
const A = function() {};
A.prototype.a = function() {
throw new Error;
};
Object.defineProperty(A.prototype.a, 'name', {value: 'SomeClass.someMethod'});
(new A).a();
11 changes: 11 additions & 0 deletions deps/v8/test/message/fail/stack-trace-method-name-configured.out
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
# Copyright 2022 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.

*%(basename)s:9: Error
throw new Error;
^
Error
at SomeClass.someMethod [as a] (*%(basename)s:9:9)
at *%(basename)s:12:9

12 changes: 12 additions & 0 deletions deps/v8/test/message/fail/stack-trace-method-name-renaming.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
// Copyright 2022 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.

// Check that the dynamic type name ('A') is not prepended to the inferred
// method name ('C.a') in error.stack output.
const A = function() {};
const C = A;
C.prototype.a = function() {
throw new Error;
};
(new A).a();
11 changes: 11 additions & 0 deletions deps/v8/test/message/fail/stack-trace-method-name-renaming.out
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
# Copyright 2022 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.

*%(basename)s:10: Error
throw new Error;
^
Error
at C.a (*%(basename)s:10:9)
at *%(basename)s:12:9

39 changes: 15 additions & 24 deletions deps/v8/test/mjsunit/stack-traces-class-fields.js
Original file line number Diff line number Diff line change
Expand Up @@ -75,16 +75,13 @@ function testClassInstantiation() {

// ReferenceError: FAIL is not defined
// at thrower
// at X.<instance_members_initializer>
// at <instance_members_initializer>
// at new X
// at testClassInstantiation
// at testTrace
testTrace(
"during class instantiation",
testClassInstantiation,
["thrower", "X.<instance_members_initializer>", "new X"],
["anonymous"]
);
'during class instantiation', testClassInstantiation,
['thrower', '<instance_members_initializer>', 'new X'], ['anonymous']);

function testClassInstantiationWithSuper() {
class Base {}
Expand All @@ -98,16 +95,14 @@ function testClassInstantiationWithSuper() {

// ReferenceError: FAIL is not defined
// at thrower
// at X.<instance_members_initializer>
// at <instance_members_initializer>
// at new X
// at testClassInstantiation
// at testTrace
testTrace(
"during class instantiation with super",
testClassInstantiationWithSuper,
["thrower", "X.<instance_members_initializer>", "new X"],
["Base", "anonymous"]
);
'during class instantiation with super', testClassInstantiationWithSuper,
['thrower', '<instance_members_initializer>', 'new X'],
['Base', 'anonymous']);

function testClassInstantiationWithSuper2() {
class Base {}
Expand All @@ -124,16 +119,14 @@ function testClassInstantiationWithSuper2() {

// ReferenceError: FAIL is not defined
// at thrower
// at X.<instance_members_initializer>
// at <instance_members_initializer>
// at new X
// at testClassInstantiation
// at testTrace
testTrace(
"during class instantiation with super2",
testClassInstantiationWithSuper2,
["thrower", "X.<instance_members_initializer>", "new X"],
["Base", "anonymous"]
);
'during class instantiation with super2', testClassInstantiationWithSuper2,
['thrower', '<instance_members_initializer>', 'new X'],
['Base', 'anonymous']);

function testClassInstantiationWithSuper3() {
class Base {
Expand All @@ -151,17 +144,15 @@ function testClassInstantiationWithSuper3() {

// ReferenceError: FAIL is not defined
// at thrower
// at X.<instance_members_initializer>
// at <instance_members_initializer>
// at new Base
// at new X
// at testClassInstantiationWithSuper3
// at testTrace
testTrace(
"during class instantiation with super3",
testClassInstantiationWithSuper3,
["thrower", "X.<instance_members_initializer>", "new Base", "new X"],
["anonymous"]
);
'during class instantiation with super3', testClassInstantiationWithSuper3,
['thrower', '<instance_members_initializer>', 'new Base', 'new X'],
['anonymous']);

function testClassFieldCall() {
class X {
Expand Down