diff --git a/packages/bun-error/index.tsx b/packages/bun-error/index.tsx index e75f76621f..ab5212bae2 100644 --- a/packages/bun-error/index.tsx +++ b/packages/bun-error/index.tsx @@ -511,13 +511,7 @@ const SourceLines = ({ ); }; -const BuildErrorSourceLines = ({ - location, - filename, -}: { - location: Location; - filename: string; -}) => { +const BuildErrorSourceLines = ({ location, filename }: { location: Location; filename: string }) => { const { line, line_text, column } = location; const sourceLines: SourceLine[] = [{ line, text: line_text }]; const buildURL = React.useCallback((line, column) => srcFileURL(filename, line, column), [srcFileURL, filename]); @@ -612,7 +606,7 @@ const NativeStackFrame = ({ const { file, function_name: functionName, - position: { line, column_start: column }, + position: { line, column }, scope, } = frame; const fileName = normalizedFilename(file, cwd); @@ -689,21 +683,21 @@ const NativeStackTrace = ({ return (
- {filename}:{position.line}:{position.column_start} + {filename}:{position.line}:{position.column} {sourceLines.length > 0 && ( @@ -715,7 +709,7 @@ const NativeStackTrace = ({ highlight={position.line} sourceLines={sourceLines} setSourceLines={setSourceLines} - highlightColumnStart={position.column_start} + highlightColumnStart={position.column} buildURL={buildURL} highlightColumnEnd={position.column_stop} > @@ -737,13 +731,7 @@ const Indent = ({ by, children }) => { ); }; -const JSException = ({ - value, - isClient = false, -}: { - value: JSExceptionType; - isClient: boolean; -}) => { +const JSException = ({ value, isClient = false }: { value: JSExceptionType; isClient: boolean }) => { const tag = isClient ? ErrorTagType.client : ErrorTagType.server; const [sourceLines, _setSourceLines] = React.useState(value?.stack?.source_lines ?? []); var message = value.message || ""; @@ -791,7 +779,7 @@ const JSException = ({ sourceLines={sourceLines} setSourceLines={setSourceLines} > - + {fancyTypeError.runtimeTypeName} @@ -853,13 +841,7 @@ const JSException = ({ } }; -const Summary = ({ - errorCount, - onClose, -}: { - errorCount: number; - onClose: () => void; -}) => { +const Summary = ({ errorCount, onClose }: { errorCount: number; onClose: () => void }) => { return (
@@ -1001,11 +983,7 @@ const Footer = ({ toMarkdown, data }) => (
); -const BuildFailureMessageContainer = ({ - messages, -}: { - messages: Message[]; -}) => { +const BuildFailureMessageContainer = ({ messages }: { messages: Message[] }) => { return (
@@ -1153,14 +1131,14 @@ export function renderRuntimeError(error: Error) { file: error[fileNameProperty] || "", position: { line: +error[lineNumberProperty] || 1, - column_start: +error[columnNumberProperty] || 1, + column: +error[columnNumberProperty] || 1, }, } as StackFrame); } else if (exception.stack && exception.stack.frames.length > 0) { exception.stack.frames[0].position.line = error[lineNumberProperty]; if (Number.isFinite(error[columnNumberProperty])) { - exception.stack.frames[0].position.column_start = error[columnNumberProperty]; + exception.stack.frames[0].position.column = error[columnNumberProperty]; } } } @@ -1214,27 +1192,27 @@ export function renderRuntimeError(error: Error) { } var frame = exception.stack.frames[frameIndex]; - const { line, column_start } = frame.position; - const remapped = remapPosition(mappings, line, column_start); + const { line, column } = frame.position; + const remapped = remapPosition(mappings, line, column); if (!remapped) return null; frame.position.line_start = frame.position.line = remapped[0]; frame.position.column_stop = frame.position.expression_stop = frame.position.expression_start = - frame.position.column_start = + frame.position.column = remapped[1]; }, console.error); } else { if (!mappings) return null; var frame = exception.stack.frames[frameIndex]; - const { line, column_start } = frame.position; - const remapped = remapPosition(mappings, line, column_start); + const { line, column } = frame.position; + const remapped = remapPosition(mappings, line, column); if (!remapped) return null; frame.position.line_start = frame.position.line = remapped[0]; frame.position.column_stop = frame.position.expression_stop = frame.position.expression_start = - frame.position.column_start = + frame.position.column = remapped[1]; } }); diff --git a/src/api/schema.d.ts b/src/api/schema.d.ts index 587e06a80d..9659f0f8d3 100644 --- a/src/api/schema.d.ts +++ b/src/api/schema.d.ts @@ -355,14 +355,8 @@ export interface StackFrame { } export interface StackFramePosition { - source_offset: int32; line: int32; - line_start: int32; - line_stop: int32; - column_start: int32; - column_stop: int32; - expression_start: int32; - expression_stop: int32; + column: int32; } export interface SourceLine { diff --git a/src/api/schema.js b/src/api/schema.js index 0b99bf24a4..8451955b9d 100644 --- a/src/api/schema.js +++ b/src/api/schema.js @@ -148,14 +148,9 @@ function encodeStackFrame(message, bb) { function decodeStackFramePosition(bb) { var result = {}; - result["source_offset"] = bb.readInt32(); result["line"] = bb.readInt32(); - result["line_start"] = bb.readInt32(); - result["line_stop"] = bb.readInt32(); - result["column_start"] = bb.readInt32(); - result["column_stop"] = bb.readInt32(); - result["expression_start"] = bb.readInt32(); - result["expression_stop"] = bb.readInt32(); + result["column"] = bb.readInt32(); + return result; } diff --git a/src/api/schema.zig b/src/api/schema.zig index 69604d2be1..30c416e153 100644 --- a/src/api/schema.zig +++ b/src/api/schema.zig @@ -1,4 +1,5 @@ const std = @import("std"); +const bun = @import("root").bun; pub const Reader = struct { const Self = @This(); @@ -423,56 +424,7 @@ pub const Api = struct { } }; - pub const StackFramePosition = packed struct { - /// source_offset - source_offset: i32 = 0, - - /// line - line: i32 = 0, - - /// line_start - line_start: i32 = 0, - - /// line_stop - line_stop: i32 = 0, - - /// column_start - column_start: i32 = 0, - - /// column_stop - column_stop: i32 = 0, - - /// expression_start - expression_start: i32 = 0, - - /// expression_stop - expression_stop: i32 = 0, - - pub fn decode(reader: anytype) anyerror!StackFramePosition { - var this = std.mem.zeroes(StackFramePosition); - - this.source_offset = try reader.readValue(i32); - this.line = try reader.readValue(i32); - this.line_start = try reader.readValue(i32); - this.line_stop = try reader.readValue(i32); - this.column_start = try reader.readValue(i32); - this.column_stop = try reader.readValue(i32); - this.expression_start = try reader.readValue(i32); - this.expression_stop = try reader.readValue(i32); - return this; - } - - pub fn encode(this: *const @This(), writer: anytype) anyerror!void { - try writer.writeInt(this.source_offset); - try writer.writeInt(this.line); - try writer.writeInt(this.line_start); - try writer.writeInt(this.line_stop); - try writer.writeInt(this.column_start); - try writer.writeInt(this.column_stop); - try writer.writeInt(this.expression_start); - try writer.writeInt(this.expression_stop); - } - }; + pub const StackFramePosition = bun.JSC.ZigStackFramePosition; pub const SourceLine = struct { /// line diff --git a/src/bun.js/bindings/BunProcess.cpp b/src/bun.js/bindings/BunProcess.cpp index e4a95fe0a6..26d5cd54e0 100644 --- a/src/bun.js/bindings/BunProcess.cpp +++ b/src/bun.js/bindings/BunProcess.cpp @@ -26,6 +26,7 @@ #include "ProcessBindingTTYWrap.h" #include "wtf/text/ASCIILiteral.h" +#include "wtf/text/OrdinalNumber.h" #ifndef WIN32 #include @@ -1591,10 +1592,14 @@ static JSValue constructReportObjectComplete(VM& vm, Zig::GlobalObject* globalOb vm.interpreter.getStackTrace(javascriptStack, stackFrames, 1); String name = "Error"_s; String message = "JavaScript Callstack"_s; - unsigned int line = 0; - unsigned int column = 0; + OrdinalNumber line = OrdinalNumber::beforeFirst(); + OrdinalNumber column = OrdinalNumber::beforeFirst(); WTF::String sourceURL; - WTF::String stackProperty = Bun::formatStackTrace(vm, globalObject, name, message, line, column, sourceURL, stackFrames, nullptr); + WTF::String stackProperty = Bun::formatStackTrace( + vm, globalObject, name, message, + line, column, + sourceURL, stackFrames, nullptr); + WTF::String stack; // first line after "Error:" size_t firstLine = stackProperty.find('\n'); diff --git a/src/bun.js/bindings/CallSite.cpp b/src/bun.js/bindings/CallSite.cpp index 018f05a479..85a41a17be 100644 --- a/src/bun.js/bindings/CallSite.cpp +++ b/src/bun.js/bindings/CallSite.cpp @@ -58,8 +58,8 @@ void CallSite::finishCreation(VM& vm, JSC::JSGlobalObject* globalObject, JSCStac const auto* sourcePositions = stackFrame.getSourcePositions(); if (sourcePositions) { - m_lineNumber = sourcePositions->line.oneBasedInt(); - m_columnNumber = sourcePositions->startColumn.oneBasedInt(); + m_lineNumber = sourcePositions->line; + m_columnNumber = sourcePositions->column; } if (stackFrame.isEval()) { @@ -105,8 +105,8 @@ void CallSite::formatAsString(JSC::VM& vm, JSC::JSGlobalObject* globalObject, WT JSString* myFunctionName = functionName().toString(globalObject); JSString* mySourceURL = sourceURL().toString(globalObject); - JSString* myColumnNumber = columnNumber() >= 0 ? JSValue(columnNumber()).toString(globalObject) : jsEmptyString(vm); - JSString* myLineNumber = lineNumber() >= 0 ? JSValue(lineNumber()).toString(globalObject) : jsEmptyString(vm); + JSString* myColumnNumber = columnNumber().zeroBasedInt() >= 0 ? JSValue(columnNumber().oneBasedInt()).toString(globalObject) : jsEmptyString(vm); + JSString* myLineNumber = lineNumber().zeroBasedInt() >= 0 ? JSValue(lineNumber().oneBasedInt()).toString(globalObject) : jsEmptyString(vm); bool myIsConstructor = isConstructor(); diff --git a/src/bun.js/bindings/CallSite.h b/src/bun.js/bindings/CallSite.h index 765f961131..35c2e42174 100644 --- a/src/bun.js/bindings/CallSite.h +++ b/src/bun.js/bindings/CallSite.h @@ -9,6 +9,7 @@ #include #include "BunClientData.h" +#include "wtf/text/OrdinalNumber.h" using namespace JSC; using namespace WebCore; @@ -31,8 +32,8 @@ private: JSC::WriteBarrier m_function; JSC::WriteBarrier m_functionName; JSC::WriteBarrier m_sourceURL; - int32_t m_lineNumber = -1; - int32_t m_columnNumber = -1; + OrdinalNumber m_lineNumber; + OrdinalNumber m_columnNumber; unsigned int m_flags; public: @@ -70,23 +71,23 @@ public: JSC::JSValue function() const { return m_function.get(); } JSC::JSValue functionName() const { return m_functionName.get(); } JSC::JSValue sourceURL() const { return m_sourceURL.get(); } - int32_t lineNumber() const { return m_lineNumber; } - int32_t columnNumber() const { return m_columnNumber; } + OrdinalNumber lineNumber() const { return m_lineNumber; } + OrdinalNumber columnNumber() const { return m_columnNumber; } bool isEval() const { return m_flags & static_cast(Flags::IsEval); } bool isConstructor() const { return m_flags & static_cast(Flags::IsConstructor); } bool isStrict() const { return m_flags & static_cast(Flags::IsStrict); } bool isNative() const { return m_flags & static_cast(Flags::IsNative); } - void setLineNumber(int32_t lineNumber) { m_lineNumber = lineNumber; } - void setColumnNumber(int32_t columnNumber) { m_columnNumber = columnNumber; } + void setLineNumber(OrdinalNumber lineNumber) { m_lineNumber = lineNumber; } + void setColumnNumber(OrdinalNumber columnNumber) { m_columnNumber = columnNumber; } void formatAsString(JSC::VM& vm, JSC::JSGlobalObject* globalObject, WTF::StringBuilder& sb); private: CallSite(JSC::VM& vm, JSC::Structure* structure) : Base(vm, structure) - , m_lineNumber(-1) - , m_columnNumber(-1) + , m_lineNumber(OrdinalNumber::beforeFirst()) + , m_columnNumber(OrdinalNumber::beforeFirst()) , m_flags(0) { } diff --git a/src/bun.js/bindings/CallSitePrototype.cpp b/src/bun.js/bindings/CallSitePrototype.cpp index 75a10f118a..0e9eb93ffd 100644 --- a/src/bun.js/bindings/CallSitePrototype.cpp +++ b/src/bun.js/bindings/CallSitePrototype.cpp @@ -139,14 +139,14 @@ JSC_DEFINE_HOST_FUNCTION(callSiteProtoFuncGetLineNumber, (JSGlobalObject * globa { ENTER_PROTO_FUNC(); // https://github.com/mozilla/source-map/blob/60adcb064bf033702d954d6d3f9bc3635dcb744b/lib/source-map-consumer.js#L484-L486 - return JSC::JSValue::encode(jsNumber(std::max(callSite->lineNumber(), 1))); + return JSC::JSValue::encode(jsNumber(std::max(callSite->lineNumber().oneBasedInt(), 1))); } JSC_DEFINE_HOST_FUNCTION(callSiteProtoFuncGetColumnNumber, (JSGlobalObject * globalObject, JSC::CallFrame* callFrame)) { ENTER_PROTO_FUNC(); // https://github.com/mozilla/source-map/blob/60adcb064bf033702d954d6d3f9bc3635dcb744b/lib/source-map-consumer.js#L488-L489 - return JSC::JSValue::encode(jsNumber(std::max(callSite->columnNumber(), 0))); + return JSC::JSValue::encode(jsNumber(std::max(callSite->columnNumber().zeroBasedInt(), 0))); } // TODO: diff --git a/src/bun.js/bindings/ErrorStackFrame.cpp b/src/bun.js/bindings/ErrorStackFrame.cpp new file mode 100644 index 0000000000..98fe90acf6 --- /dev/null +++ b/src/bun.js/bindings/ErrorStackFrame.cpp @@ -0,0 +1,91 @@ +#include "root.h" +#include "JavaScriptCore/CodeBlock.h" +#include "headers-handwritten.h" +#include "JavaScriptCore/BytecodeIndex.h" +#include "wtf/Assertions.h" +#include "wtf/text/OrdinalNumber.h" + +namespace Bun { +using namespace JSC; + +/// Adjust a `ZigStackFramePosition` by a number of bytes. This accounts for when the adjustment +/// crosses line boundaries, and thus requires the source code in order to properly compute +/// the result. +void adjustPositionBackwards(ZigStackFramePosition& pos, int amount, CodeBlock* code) +{ + if (pos.byte_position - amount < 0) { + pos.line_zero_based = 0; + pos.column_zero_based = 0; + pos.byte_position = 0; + return; + } + + pos.column_zero_based = pos.column_zero_based - amount; + if (pos.column_zero_based < 0) { + auto source = code->source().provider()->source(); + if (!source.is8Bit()) { + // Debug-only assertion + // Bun does not yet use 16-bit sources anywhere. The transpiler ensures everything + // fit's into latin1 / 8-bit strings for on-average lower memory usage. + ASSERT_NOT_REACHED("16-bit source re-mapping is not implemented here."); + + pos.line_zero_based = 0; + pos.column_zero_based = 0; + pos.byte_position = 0; + return; + } + + for (int i = 0; i < amount; i++) { + if (source[pos.byte_position - i] == '\n') { + pos.line_zero_based = pos.line_zero_based - 1; + } + } + + int columns = 0; + // Initial -1 to skip the newline that gets counted. + int i = pos.byte_position - amount - 1; + while (i > 0 && source[i] != '\n') { + columns += 1; + i -= 1; + } + pos.column_zero_based = columns; + } + + pos.byte_position -= amount; +} + +ZigStackFramePosition getAdjustedPositionForBytecode(JSC::CodeBlock* code, JSC::BytecodeIndex bc) +{ + auto expr = code->expressionInfoForBytecodeIndex(bc); + + ZigStackFramePosition pos { + .line_zero_based = OrdinalNumber::fromOneBasedInt(expr.lineColumn.line).zeroBasedInt(), + .column_zero_based = OrdinalNumber::fromOneBasedInt(expr.lineColumn.column).zeroBasedInt(), + .byte_position = (int)expr.divot, + }; + + auto inst = code->instructionAt(bc); + + /// JavaScriptCore places error divots at different places than v8 + // Uncomment to debug this: + // printf("lc = %d : %d (byte = %d)\n", pos.line.oneBasedInt(), pos.column.oneBasedInt(), expr.divot); + // printf("off = %d : %d\n", expr.startOffset, expr.endOffset); + // printf("name = %s\n", inst->name()); + + switch (inst->opcodeID()) { + case op_construct: + case op_construct_varargs: + // The divot by default is pointing at the `(` or the end of the class name. + // We want to point at the `new` keyword, which is conveniently at the + // expression start. + adjustPositionBackwards(pos, expr.startOffset, code); + break; + + default: + break; + } + + return pos; +} + +} // namespace Bun \ No newline at end of file diff --git a/src/bun.js/bindings/ErrorStackFrame.h b/src/bun.js/bindings/ErrorStackFrame.h new file mode 100644 index 0000000000..d7a04e83d6 --- /dev/null +++ b/src/bun.js/bindings/ErrorStackFrame.h @@ -0,0 +1,9 @@ +#include "root.h" +#include "headers-handwritten.h" +#include "JavaScriptCore/BytecodeIndex.h" + +namespace Bun { + +ZigStackFramePosition getAdjustedPositionForBytecode(JSC::CodeBlock* code, JSC::BytecodeIndex bc); + +} // namespace Bun diff --git a/src/bun.js/bindings/ErrorStackTrace.cpp b/src/bun.js/bindings/ErrorStackTrace.cpp index 8af50c1126..ae2e282c1d 100644 --- a/src/bun.js/bindings/ErrorStackTrace.cpp +++ b/src/bun.js/bindings/ErrorStackTrace.cpp @@ -5,6 +5,8 @@ #include "config.h" #include "ErrorStackTrace.h" +#include "JavaScriptCore/Error.h" +#include "wtf/text/OrdinalNumber.h" #include #include @@ -15,6 +17,8 @@ #include #include +#include "ErrorStackFrame.h" + using namespace JSC; using namespace WebCore; @@ -358,65 +362,13 @@ bool JSCStackFrame::calculateSourcePositions() if (!m_codeBlock) { return false; } - - JSC::BytecodeIndex bytecodeIndex = hasBytecodeIndex() ? m_bytecodeIndex : JSC::BytecodeIndex(); - - /* Get the "raw" position info. - * Note that we're using m_codeBlock->unlinkedCodeBlock()->expressionRangeForBytecodeOffset rather than m_codeBlock->expressionRangeForBytecodeOffset - * in order get the "raw" offsets and avoid the CodeBlock's expressionRangeForBytecodeOffset modifications to the line and column numbers, - * (we don't need the column number from it, and we'll calculate the line "fixes" ourselves). */ - ExpressionInfo::Entry info = m_codeBlock->unlinkedCodeBlock()->expressionInfoForBytecodeIndex(bytecodeIndex); - info.divot += m_codeBlock->sourceOffset(); - - /* On the first line of the source code, it seems that we need to "fix" the column with the starting - * offset. We currently use codeBlock->source()->startPosition().m_column.oneBasedInt() as the - * offset in the first line rather than codeBlock->firstLineColumnOffset(), which seems simpler - * (and what CodeBlock::expressionRangeForBytecodeOffset does). This is because firstLineColumnOffset - * values seems different from what we expect (according to v8's tests) and I haven't dove into the - * relevant parts in JSC (yet) to figure out why. */ - unsigned columnOffset = info.lineColumn.line ? 0 : m_codeBlock->source().startColumn().zeroBasedInt(); - - // "Fix" the line number - JSC::ScriptExecutable* executable = m_codeBlock->ownerExecutable(); - info.lineColumn.line = executable->overrideLineNumber(m_vm).value_or(info.lineColumn.line + executable->firstLine()); - - // Calculate the staring\ending offsets of the entire expression - int expressionStart = info.divot - info.startOffset; - int expressionStop = info.divot + info.endOffset; - - // Make sure the range is valid - StringView sourceString = m_codeBlock->source().provider()->source(); - if (!expressionStop || expressionStart > static_cast(sourceString.length())) { + if (!hasBytecodeIndex()) { return false; } - // Search for the beginning of the line - unsigned int lineStart = expressionStart; - while ((lineStart > 0) && ('\n' != sourceString[lineStart - 1])) { - lineStart--; - } - // Search for the end of the line - unsigned int lineStop = expressionStop; - unsigned int sourceLength = sourceString.length(); - while ((lineStop < sourceLength) && ('\n' != sourceString[lineStop])) { - lineStop++; - } - - /* Finally, store the source "positions" info. - * Notes: - * - The retrieved column seem to point the "end column". To make sure we're current, we'll calculate the - * columns ourselves, since we've already found where the line starts. Note that in v8 it should be 0-based - * here (in contrast the 1-based column number in v8::StackFrame). - * - The static_casts are ugly, but comes from differences between JSC and v8's api, and should be OK - * since no source should be longer than "max int" chars. - */ - m_sourcePositions.expressionStart = WTF::OrdinalNumber::fromZeroBasedInt(expressionStart); - m_sourcePositions.expressionStop = WTF::OrdinalNumber::fromZeroBasedInt(expressionStop); - m_sourcePositions.line = WTF::OrdinalNumber::fromZeroBasedInt(static_cast(info.lineColumn.line)); - m_sourcePositions.startColumn = WTF::OrdinalNumber::fromZeroBasedInt((expressionStart - lineStart) + columnOffset); - m_sourcePositions.endColumn = WTF::OrdinalNumber::fromZeroBasedInt(m_sourcePositions.startColumn.zeroBasedInt() + (expressionStop - expressionStart)); - m_sourcePositions.lineStart = WTF::OrdinalNumber::fromZeroBasedInt(static_cast(lineStart)); - m_sourcePositions.lineStop = WTF::OrdinalNumber::fromZeroBasedInt(static_cast(lineStop)); + auto location = Bun::getAdjustedPositionForBytecode(m_codeBlock, m_bytecodeIndex); + m_sourcePositions.line = location.line(); + m_sourcePositions.column = location.column(); return true; } diff --git a/src/bun.js/bindings/ErrorStackTrace.h b/src/bun.js/bindings/ErrorStackTrace.h index ac40eaf4bc..e33cc18a6e 100644 --- a/src/bun.js/bindings/ErrorStackTrace.h +++ b/src/bun.js/bindings/ErrorStackTrace.h @@ -18,7 +18,7 @@ namespace Zig { /* JSCStackFrame is an alternative to JSC::StackFrame, which provides the following advantages\changes: * - Also hold the call frame (ExecState). This is mainly used by CallSite to get "this value". - * - More detailed and v8 compatible "source offsets" caculations: JSC::StackFrame only provides the + * - More detailed and v8 compatible "source offsets" calculations: JSC::StackFrame only provides the * line number and column numbers. It's column calculation seems to be different than v8's column. * According to v8's unit tests, it seems that their column number points to the beginning of * the expression which raised the exception, while in JSC the column returned by computeLineAndColumn @@ -34,19 +34,14 @@ namespace Zig { * - String properties are exposed (and cached) as JSStrings, instead of WTF::String. * - Helper functions like isEval and isConstructor. * - * Note that this is not a heap allocated, garbage collected, JSCell. It must be stack allocated, as it doens't + * Note that this is not a heap allocated, garbage collected, JSCell. It must be stack allocated, as it doesn't * use any write barriers and rely on the GC to see the stored JSC object pointers on the stack. */ class JSCStackFrame { public: struct SourcePositions { - WTF::OrdinalNumber expressionStart; - WTF::OrdinalNumber expressionStop; WTF::OrdinalNumber line; - WTF::OrdinalNumber startColumn; - WTF::OrdinalNumber endColumn; - WTF::OrdinalNumber lineStart; - WTF::OrdinalNumber lineStop; + WTF::OrdinalNumber column; }; private: @@ -97,7 +92,7 @@ public: return m_bytecodeIndex; } - // Returns null if can't retreive the source positions + // Returns null if can't retrieve the source positions SourcePositions* getSourcePositions(); bool isWasmFrame() const { return m_isWasmFrame; } @@ -111,7 +106,7 @@ private: * the same logic, which is to first try the function's "display name", and if it's not defined, * the function's name. In JSC, StackFrame::functionName uses JSC::getCalculatedDisplayName, * which will internally call the JSFunction\InternalFunction's calculatedDisplayName function. - * But, those function don't check the function's "name" property if the "dispaly name" isn't defined. + * But, those function don't check the function's "name" property if the "display name" isn't defined. * See JSFunction::name()'s and InternalFunction::name()'s implementation. According to v8's unit tests, * v8 does check the name property in StackFrame::GetFunctionName (see the last part of the * "CaptureStackTrace" test in test-api.cc). diff --git a/src/bun.js/bindings/ZigGlobalObject.cpp b/src/bun.js/bindings/ZigGlobalObject.cpp index c8072c9745..7de6e5ffa6 100644 --- a/src/bun.js/bindings/ZigGlobalObject.cpp +++ b/src/bun.js/bindings/ZigGlobalObject.cpp @@ -147,6 +147,8 @@ #include "ZigSourceProvider.h" #include "UtilInspect.h" #include "Base64Helpers.h" +#include "wtf/text/OrdinalNumber.h" + #if ENABLE(REMOTE_INSPECTOR) #include "JavaScriptCore/RemoteInspectorServer.h" #endif @@ -300,14 +302,14 @@ static JSValue formatStackTraceToJSValue(JSC::VM& vm, Zig::GlobalObject* globalO } } - bool orignialSkipNextComputeErrorInfo = skipNextComputeErrorInfo; + bool originalSkipNextComputeErrorInfo = skipNextComputeErrorInfo; skipNextComputeErrorInfo = true; if (errorObject->hasProperty(lexicalGlobalObject, vm.propertyNames->stack)) { skipNextComputeErrorInfo = true; errorObject->deleteProperty(lexicalGlobalObject, vm.propertyNames->stack); } - skipNextComputeErrorInfo = orignialSkipNextComputeErrorInfo; + skipNextComputeErrorInfo = originalSkipNextComputeErrorInfo; JSValue stackStringValue = jsString(vm, sb.toString()); @@ -344,7 +346,16 @@ static JSValue formatStackTraceToJSValue(JSC::VM& vm, Zig::GlobalObject* globalO return stackStringValue; } -WTF::String Bun::formatStackTrace(JSC::VM& vm, JSC::JSGlobalObject* globalObject, const WTF::String& name, const WTF::String& message, unsigned& line, unsigned& column, WTF::String& sourceURL, Vector& stackTrace, JSC::JSObject* errorInstance) +WTF::String Bun::formatStackTrace( + JSC::VM& vm, + JSC::JSGlobalObject* globalObject, + const WTF::String& name, + const WTF::String& message, + OrdinalNumber& line, + OrdinalNumber& column, + WTF::String& sourceURL, + Vector& stackTrace, + JSC::JSObject* errorInstance) { WTF::StringBuilder sb; @@ -376,8 +387,8 @@ WTF::String Bun::formatStackTrace(JSC::VM& vm, JSC::JSGlobalObject* globalObject ZigStackFrame remappedFrame = {}; memset(&remappedFrame, 0, sizeof(ZigStackFrame)); - remappedFrame.position.line = originalLine.zeroBasedInt() + 1; - remappedFrame.position.column_start = 0; + remappedFrame.position.line_zero_based = originalLine.zeroBasedInt(); + remappedFrame.position.column_zero_based = 0; String sourceURLForFrame = err->sourceURL(); @@ -405,12 +416,12 @@ WTF::String Bun::formatStackTrace(JSC::VM& vm, JSC::JSGlobalObject* globalObject if (remappedFrame.remapped) { errorInstance->putDirect(vm, Identifier::fromString(vm, "originalLine"_s), jsNumber(originalLine.oneBasedInt()), 0); hasSet = true; - line = remappedFrame.position.line; + line = remappedFrame.position.line(); } if (remappedFrame.remapped) { sb.append(":"_s); - sb.append(remappedFrame.position.line); + sb.append(remappedFrame.position.line().oneBasedInt()); } else { sb.append(":"_s); sb.append(originalLine.oneBasedInt()); @@ -474,14 +485,12 @@ WTF::String Bun::formatStackTrace(JSC::VM& vm, JSC::JSGlobalObject* globalObject } if (frame.hasLineAndColumnInfo()) { - unsigned int thisLine = 0; - unsigned int thisColumn = 0; LineColumn lineColumn = frame.computeLineAndColumn(); - thisLine = lineColumn.line; - thisColumn = lineColumn.column; + OrdinalNumber thisLine = OrdinalNumber::fromOneBasedInt(lineColumn.line); + OrdinalNumber thisColumn = OrdinalNumber::fromOneBasedInt(lineColumn.column); ZigStackFrame remappedFrame = {}; - remappedFrame.position.line = thisLine; - remappedFrame.position.column_start = thisColumn; + remappedFrame.position.line_zero_based = thisLine.zeroBasedInt(); + remappedFrame.position.column_zero_based = thisColumn.zeroBasedInt(); String sourceURLForFrame = frame.sourceURL(vm); @@ -506,8 +515,8 @@ WTF::String Bun::formatStackTrace(JSC::VM& vm, JSC::JSGlobalObject* globalObject if (remappedFrame.remapped) { if (errorInstance) { - errorInstance->putDirect(vm, Identifier::fromString(vm, "originalLine"_s), jsNumber(thisLine), 0); - errorInstance->putDirect(vm, Identifier::fromString(vm, "originalColumn"_s), jsNumber(thisColumn), 0); + errorInstance->putDirect(vm, Identifier::fromString(vm, "originalLine"_s), jsNumber(thisLine.oneBasedInt()), 0); + errorInstance->putDirect(vm, Identifier::fromString(vm, "originalColumn"_s), jsNumber(thisColumn.oneBasedInt()), 0); } } } @@ -515,9 +524,9 @@ WTF::String Bun::formatStackTrace(JSC::VM& vm, JSC::JSGlobalObject* globalObject sb.append(" ("_s); sb.append(sourceURLForFrame); sb.append(":"_s); - sb.append(remappedFrame.position.line); + sb.append(remappedFrame.position.line().oneBasedInt()); sb.append(":"_s); - sb.append(remappedFrame.position.column_start); + sb.append(remappedFrame.position.column().oneBasedInt()); sb.append(")"_s); } else { sb.append(" (native)"_s); @@ -532,7 +541,14 @@ WTF::String Bun::formatStackTrace(JSC::VM& vm, JSC::JSGlobalObject* globalObject } // error.stack calls this function -static String computeErrorInfoWithoutPrepareStackTrace(JSC::VM& vm, Zig::GlobalObject* globalObject, Vector& stackTrace, unsigned& line, unsigned& column, String& sourceURL, JSObject* errorInstance) +static String computeErrorInfoWithoutPrepareStackTrace( + JSC::VM& vm, + Zig::GlobalObject* globalObject, + Vector& stackTrace, + OrdinalNumber& line, + OrdinalNumber& column, + String& sourceURL, + JSObject* errorInstance) { WTF::String name = "Error"_s; @@ -554,7 +570,7 @@ static String computeErrorInfoWithoutPrepareStackTrace(JSC::VM& vm, Zig::GlobalO return Bun::formatStackTrace(vm, globalObject, name, message, line, column, sourceURL, stackTrace, errorInstance); } -static String computeErrorInfoWithPrepareStackTrace(JSC::VM& vm, Zig::GlobalObject* globalObject, JSC::JSGlobalObject* lexicalGlobalObject, Vector& stackFrames, unsigned& line, unsigned& column, String& sourceURL, JSObject* errorObject, JSObject* prepareStackTrace) +static String computeErrorInfoWithPrepareStackTrace(JSC::VM& vm, Zig::GlobalObject* globalObject, JSC::JSGlobalObject* lexicalGlobalObject, Vector& stackFrames, OrdinalNumber& line, OrdinalNumber& column, String& sourceURL, JSObject* errorObject, JSObject* prepareStackTrace) { auto scope = DECLARE_THROW_SCOPE(vm); @@ -576,11 +592,11 @@ static String computeErrorInfoWithPrepareStackTrace(JSC::VM& vm, Zig::GlobalObje remappedFrames[i] = {}; remappedFrames[i].source_url = Bun::toString(lexicalGlobalObject, stackTrace.at(i).sourceURL()); if (JSCStackFrame::SourcePositions* sourcePositions = stackTrace.at(i).getSourcePositions()) { - remappedFrames[i].position.line = sourcePositions->line.oneBasedInt(); - remappedFrames[i].position.column_start = sourcePositions->startColumn.oneBasedInt() + 1; + remappedFrames[i].position.line_zero_based = sourcePositions->line.zeroBasedInt(); + remappedFrames[i].position.column_zero_based = sourcePositions->column.zeroBasedInt(); } else { - remappedFrames[i].position.line = -1; - remappedFrames[i].position.column_start = -1; + remappedFrames[i].position.line_zero_based = -1; + remappedFrames[i].position.column_zero_based = -1; } } @@ -590,11 +606,8 @@ static String computeErrorInfoWithPrepareStackTrace(JSC::VM& vm, Zig::GlobalObje JSC::JSValue callSiteValue = callSites->getIndex(lexicalGlobalObject, i); CallSite* callSite = JSC::jsDynamicCast(callSiteValue); if (remappedFrames[i].remapped) { - int32_t remappedColumnStart = remappedFrames[i].position.column_start; - callSite->setColumnNumber(remappedColumnStart); - - int32_t remappedLine = remappedFrames[i].position.line; - callSite->setLineNumber(remappedLine); + callSite->setColumnNumber(remappedFrames[i].position.column()); + callSite->setLineNumber(remappedFrames[i].position.line()); } } } @@ -614,12 +627,15 @@ static String computeErrorInfoWithPrepareStackTrace(JSC::VM& vm, Zig::GlobalObje return String(); } -static String computeErrorInfo(JSC::VM& vm, Vector& stackTrace, unsigned& line, unsigned& column, String& sourceURL, JSObject* errorInstance) +static String computeErrorInfo(JSC::VM& vm, Vector& stackTrace, unsigned& line_in, unsigned& column_in, String& sourceURL, JSObject* errorInstance) { if (skipNextComputeErrorInfo) { return String(); } + OrdinalNumber line = OrdinalNumber::fromOneBasedInt(line_in); + OrdinalNumber column = OrdinalNumber::fromOneBasedInt(column_in); + Zig::GlobalObject* globalObject = nullptr; if (errorInstance) { @@ -2467,11 +2483,11 @@ JSC_DEFINE_HOST_FUNCTION(errorConstructorFuncCaptureStackTrace, (JSC::JSGlobalOb memset(remappedFrames + i, 0, sizeof(ZigStackFrame)); remappedFrames[i].source_url = Bun::toString(lexicalGlobalObject, stackTrace.at(i).sourceURL()); if (JSCStackFrame::SourcePositions* sourcePositions = stackTrace.at(i).getSourcePositions()) { - remappedFrames[i].position.line = sourcePositions->line.oneBasedInt(); - remappedFrames[i].position.column_start = sourcePositions->startColumn.oneBasedInt() + 1; + remappedFrames[i].position.line_zero_based = sourcePositions->line.zeroBasedInt(); + remappedFrames[i].position.column_zero_based = sourcePositions->column.zeroBasedInt(); } else { - remappedFrames[i].position.line = -1; - remappedFrames[i].position.column_start = -1; + remappedFrames[i].position.line_zero_based = -1; + remappedFrames[i].position.column_zero_based = -1; } } @@ -2485,11 +2501,8 @@ JSC_DEFINE_HOST_FUNCTION(errorConstructorFuncCaptureStackTrace, (JSC::JSGlobalOb JSC::JSValue callSiteValue = callSites->getIndex(lexicalGlobalObject, i); CallSite* callSite = JSC::jsDynamicCast(callSiteValue); if (remappedFrames[i].remapped) { - int32_t remappedColumnStart = remappedFrames[i].position.column_start; - callSite->setColumnNumber(remappedColumnStart); - - int32_t remappedLine = remappedFrames[i].position.line; - callSite->setLineNumber(remappedLine); + callSite->setColumnNumber(remappedFrames[i].position.column()); + callSite->setLineNumber(remappedFrames[i].position.line()); } } diff --git a/src/bun.js/bindings/ZigGlobalObject.h b/src/bun.js/bindings/ZigGlobalObject.h index 360f3a60d1..242b1917f0 100644 --- a/src/bun.js/bindings/ZigGlobalObject.h +++ b/src/bun.js/bindings/ZigGlobalObject.h @@ -588,7 +588,7 @@ public: // TODO: move this namespace Bun { -String formatStackTrace(JSC::VM& vm, JSC::JSGlobalObject* globalObject, const WTF::String& name, const WTF::String& message, unsigned& line, unsigned& column, WTF::String& sourceURL, Vector& stackTrace, JSC::JSObject* errorInstance); +String formatStackTrace(JSC::VM& vm, JSC::JSGlobalObject* globalObject, const WTF::String& name, const WTF::String& message, OrdinalNumber& line, OrdinalNumber& column, WTF::String& sourceURL, Vector& stackTrace, JSC::JSObject* errorInstance); } diff --git a/src/bun.js/bindings/bindings.cpp b/src/bun.js/bindings/bindings.cpp index 3d7220da60..0c1bee9933 100644 --- a/src/bun.js/bindings/bindings.cpp +++ b/src/bun.js/bindings/bindings.cpp @@ -1,4 +1,3 @@ - #include "root.h" #include "JavaScriptCore/JSCJSValue.h" @@ -53,6 +52,7 @@ #include "wtf/Assertions.h" #include "wtf/text/ExternalStringImpl.h" +#include "wtf/text/OrdinalNumber.h" #include "wtf/text/StringCommon.h" #include "wtf/text/StringImpl.h" #include "wtf/text/StringView.h" @@ -111,6 +111,8 @@ #include "JavaScriptCore/GetterSetter.h" #include "JavaScriptCore/CustomGetterSetter.h" +#include "ErrorStackFrame.h" + static WTF::StringView StringView_slice(WTF::StringView sv, unsigned start, unsigned end) { return sv.substring(start, end - start); @@ -3898,83 +3900,52 @@ static void populateStackFrameMetadata(JSC::VM& vm, const JSC::StackFrame* stack frame->function_name = Bun::toStringRef(JSC::getCalculatedDisplayName(vm, callee)); } } -// Based on -// https://github.com/mceSystems/node-jsc/blob/master/deps/jscshim/src/shim/JSCStackTrace.cpp#L298 + static void populateStackFramePosition(const JSC::StackFrame* stackFrame, BunString* source_lines, - int32_t* source_line_numbers, uint8_t source_lines_count, + OrdinalNumber* source_line_numbers, uint8_t source_lines_count, ZigStackFramePosition* position) { - auto m_codeBlock = stackFrame->codeBlock(); - if (!m_codeBlock) + auto code = stackFrame->codeBlock(); + if (!code) return; - // https://github.com/oven-sh/bun/issues/6951 - auto* provider = m_codeBlock->source().provider(); - + auto* provider = code->source().provider(); if (UNLIKELY(!provider)) return; - - // Make sure the range is valid + // Make sure the range is valid: + // https://github.com/oven-sh/bun/issues/6951 WTF::StringView sourceString = provider->source(); - if (UNLIKELY(sourceString.isNull())) return; - - JSC::BytecodeIndex bytecodeOffset = stackFrame->hasBytecodeIndex() ? stackFrame->bytecodeIndex() : JSC::BytecodeIndex(); - - /* Get the "raw" position info. - * Note that we're using m_codeBlock->unlinkedCodeBlock()->expressionRangeForBytecodeOffset - * rather than m_codeBlock->expressionRangeForBytecodeOffset in order get the "raw" offsets and - * avoid the CodeBlock's expressionRangeForBytecodeOffset modifications to the line and column - * numbers, (we don't need the column number from it, and we'll calculate the line "fixes" - * ourselves). */ - ExpressionInfo::Entry info = m_codeBlock->unlinkedCodeBlock()->expressionInfoForBytecodeIndex(bytecodeOffset); - info.divot += m_codeBlock->sourceOffset(); - - // TODO: evaluate if using the API from UnlinkedCodeBlock can be used instead of iterating - // through source text. - - /* On the first line of the source code, it seems that we need to "fix" the column with the - * starting offset. We currently use codeBlock->source()->startPosition().m_column.oneBasedInt() - * as the offset in the first line rather than codeBlock->firstLineColumnOffset(), which seems - * simpler (and what CodeBlock::expressionRangeForBytecodeOffset does). This is because - * firstLineColumnOffset values seems different from what we expect (according to v8's tests) - * and I haven't dove into the relevant parts in JSC (yet) to figure out why. */ - unsigned columnOffset = info.lineColumn.line ? 0 : m_codeBlock->source().startColumn().zeroBasedInt(); - - // "Fix" the line number - JSC::ScriptExecutable* executable = m_codeBlock->ownerExecutable(); - if (std::optional overrideLine = executable->overrideLineNumber(m_codeBlock->vm())) { - info.lineColumn.line = overrideLine.value(); - } else { - info.lineColumn.line += executable->firstLine(); - } - - // Calculate the staring\ending offsets of the entire expression - int expressionStart = info.divot - info.startOffset; - int expressionStop = info.divot + info.endOffset; - if (expressionStop < 1 || expressionStart > static_cast(sourceString.length())) { + if (!stackFrame->hasBytecodeIndex()) { + auto lineColumn = stackFrame->computeLineAndColumn(); + position->line_zero_based = OrdinalNumber::fromOneBasedInt(lineColumn.line).zeroBasedInt(); + position->column_zero_based = OrdinalNumber::fromOneBasedInt(lineColumn.column).zeroBasedInt(); + position->byte_position = -1; return; } - // Search for the beginning of the line - unsigned int lineStart = expressionStart > 0 ? expressionStart : 0; - while ((lineStart > 0) && ('\n' != sourceString[lineStart - 1])) { - lineStart--; - } - // Search for the end of the line - unsigned int lineStop = expressionStop; - unsigned int sourceLength = sourceString.length(); - while ((lineStop < sourceLength) && ('\n' != sourceString[lineStop])) { - lineStop++; - } + auto location = Bun::getAdjustedPositionForBytecode(code, stackFrame->bytecodeIndex()); + if (source_lines_count > 1 && source_lines != nullptr && sourceString.is8Bit()) { - auto chars = sourceString.span8().data(); + // Search for the beginning of the line + unsigned int lineStart = location.byte_position; + while (lineStart > 0 && sourceString[lineStart] != '\n') { + lineStart--; + } + + // Search for the end of the line + unsigned int lineEnd = location.byte_position; + unsigned int maxSearch = sourceString.length(); + while (lineEnd < maxSearch && sourceString[lineEnd] != '\n') { + lineEnd++; + } + + const unsigned char* bytes = sourceString.span8().data(); // Most of the time, when you look at a stack trace, you want a couple lines above - - source_lines[0] = Bun::toStringRef(sourceString.substring(lineStart, lineStop - lineStart).toStringWithoutCopying()); - source_line_numbers[0] = info.lineColumn.line - 1; + source_lines[0] = Bun::toStringRef(sourceString.substring(lineStart, lineEnd - lineStart).toStringWithoutCopying()); + source_line_numbers[0] = location.line(); if (lineStart > 0) { auto byte_offset_in_source_string = lineStart - 1; @@ -3983,9 +3954,10 @@ static void populateStackFramePosition(const JSC::StackFrame* stackFrame, BunStr { // This should probably be code points instead of newlines - while (byte_offset_in_source_string > 0 && chars[byte_offset_in_source_string] != '\n') { + while (byte_offset_in_source_string > 0 && bytes[byte_offset_in_source_string] != '\n') { byte_offset_in_source_string--; } + byte_offset_in_source_string -= byte_offset_in_source_string > 0; } @@ -3993,14 +3965,16 @@ static void populateStackFramePosition(const JSC::StackFrame* stackFrame, BunStr unsigned int end_of_line_offset = byte_offset_in_source_string; // This should probably be code points instead of newlines - while (byte_offset_in_source_string > 0 && chars[byte_offset_in_source_string] != '\n') { + while (byte_offset_in_source_string > 0 && bytes[byte_offset_in_source_string] != '\n') { byte_offset_in_source_string--; } // We are at the beginning of the line - source_lines[source_line_i] = Bun::toStringRef(sourceString.substring(byte_offset_in_source_string, end_of_line_offset - byte_offset_in_source_string + 1).toStringWithoutCopying()); + source_lines[source_line_i] = Bun::toStringRef( + sourceString.substring(byte_offset_in_source_string, end_of_line_offset - byte_offset_in_source_string + 1) + .toStringWithoutCopying()); - source_line_numbers[source_line_i] = info.lineColumn.line - source_line_i - 1; + source_line_numbers[source_line_i] = location.line().fromZeroBasedInt(location.line().zeroBasedInt() - source_line_i); source_line_i++; remaining_lines_to_grab--; @@ -4010,26 +3984,9 @@ static void populateStackFramePosition(const JSC::StackFrame* stackFrame, BunStr } } - /* Finally, store the source "positions" info. - * Notes: - * - The retrieved column seem to point the "end column". To make sure we're current, we'll - *calculate the columns ourselves, since we've already found where the line starts. Note that in - *v8 it should be 0-based here (in contrast the 1-based column number in v8::StackFrame). - * - The static_casts are ugly, but comes from differences between JSC and v8's api, and should - *be OK since no source should be longer than "max int" chars. - * TODO: If expressionStart == expressionStop, then m_endColumn will be equal to m_startColumn. - *Should we handle this case? - */ - position->expression_start = expressionStart; - position->expression_stop = expressionStop; - position->line = WTF::OrdinalNumber::fromOneBasedInt(static_cast(info.lineColumn.line)).zeroBasedInt(); - position->column_start = (expressionStart - lineStart) + columnOffset; - position->column_stop = position->column_start + (expressionStop - expressionStart); - position->line_start = lineStart; - position->line_stop = lineStop; - - return; + *position = location; } + static void populateStackFrame(JSC::VM& vm, ZigStackTrace* trace, const JSC::StackFrame* stackFrame, ZigStackFrame* frame, bool is_top) { @@ -4314,9 +4271,8 @@ static void fromErrorInstance(ZigException* except, JSC::JSGlobalObject* global, String sourceURL = frame.sourceURL.toString(); current.function_name = Bun::toStringRef(functionName); current.source_url = Bun::toStringRef(sourceURL); - current.position.line = frame.lineNumber.zeroBasedInt(); - current.position.column_start = frame.columnNumber.zeroBasedInt(); - current.position.column_stop = frame.columnNumber.zeroBasedInt(); + current.position.line_zero_based = frame.lineNumber.zeroBasedInt(); + current.position.column_zero_based = frame.columnNumber.zeroBasedInt(); current.remapped = true; @@ -4345,17 +4301,17 @@ static void fromErrorInstance(ZigException* except, JSC::JSGlobalObject* global, except->stack.frames_ptr[0].source_url = Bun::toStringRef(global, sourceURL); if (JSC::JSValue column = obj->getIfPropertyExists(global, vm.propertyNames->column)) { - except->stack.frames_ptr[0].position.column_start = column.toInt32(global); + except->stack.frames_ptr[0].position.column_zero_based = OrdinalNumber::fromOneBasedInt(column.toInt32(global)).zeroBasedInt(); } if (JSC::JSValue line = obj->getIfPropertyExists(global, vm.propertyNames->line)) { - except->stack.frames_ptr[0].position.line = line.toInt32(global); + except->stack.frames_ptr[0].position.line_zero_based = OrdinalNumber::fromOneBasedInt(line.toInt32(global)).zeroBasedInt(); if (JSC::JSValue lineText = obj->getIfPropertyExists(global, JSC::Identifier::fromString(vm, "lineText"_s))) { if (JSC::JSString* jsStr = lineText.toStringOrNull(global)) { auto str = jsStr->value(global); except->stack.source_lines_ptr[0] = Bun::toStringRef(str); - except->stack.source_lines_numbers[0] = except->stack.frames_ptr[0].position.line; + except->stack.source_lines_numbers[0] = except->stack.frames_ptr[0].position.line(); except->stack.source_lines_len = 1; except->remapped = true; } @@ -4417,9 +4373,9 @@ void exceptionFromString(ZigException* except, JSC::JSValue value, JSC::JSGlobal if (line) { // TODO: don't sourcemap it twice if (auto originalLine = obj->getIfPropertyExists(global, JSC::Identifier::fromString(global->vm(), "originalLine"_s))) { - except->stack.frames_ptr[0].position.line = originalLine.toInt32(global); + except->stack.frames_ptr[0].position.line_zero_based = OrdinalNumber::fromOneBasedInt(originalLine.toInt32(global)).zeroBasedInt(); } else { - except->stack.frames_ptr[0].position.line = line.toInt32(global); + except->stack.frames_ptr[0].position.line_zero_based = OrdinalNumber::fromOneBasedInt(line.toInt32(global)).zeroBasedInt(); } except->stack.frames_len = 1; } diff --git a/src/bun.js/bindings/exports.zig b/src/bun.js/bindings/exports.zig index 8c47d69743..f71e8e32e8 100644 --- a/src/bun.js/bindings/exports.zig +++ b/src/bun.js/bindings/exports.zig @@ -553,20 +553,10 @@ pub const ZigStackFrame = extern struct { } if (!this.source_url.isEmpty()) { - frame.file = try std.fmt.allocPrint(allocator, "{any}", .{this.sourceURLFormatter(root_path, origin, true, false)}); + frame.file = try std.fmt.allocPrint(allocator, "{}", .{this.sourceURLFormatter(root_path, origin, true, false)}); } - frame.position.source_offset = this.position.source_offset; - - // For remapped code, we add 1 to the line number - frame.position.line = this.position.line + @as(i32, @intFromBool(this.remapped)); - - frame.position.line_start = this.position.line_start; - frame.position.line_stop = this.position.line_stop; - frame.position.column_start = this.position.column_start; - frame.position.column_stop = this.position.column_stop; - frame.position.expression_start = this.position.expression_start; - frame.position.expression_stop = this.position.expression_stop; + frame.position = this.position; frame.scope = @as(Api.StackFrameScope, @enumFromInt(@intFromEnum(this.code_type))); return frame; @@ -580,6 +570,7 @@ pub const ZigStackFrame = extern struct { exclude_line_column: bool = false, remapped: bool = false, root_path: string = "", + pub fn format(this: SourceURLFormatter, comptime _: []const u8, _: std.fmt.FormatOptions, writer: anytype) !void { if (this.enable_color) { try writer.writeAll(Output.prettyFmt("", true)); @@ -607,7 +598,7 @@ pub const ZigStackFrame = extern struct { try writer.writeAll(source_slice); if (this.enable_color) { - if (this.position.line > -1) { + if (this.position.line.isValid()) { try writer.writeAll(comptime Output.prettyFmt("", true)); } else { try writer.writeAll(comptime Output.prettyFmt("", true)); @@ -615,29 +606,31 @@ pub const ZigStackFrame = extern struct { } if (!this.exclude_line_column) { - if (this.position.line > -1 and this.position.column_start > -1) { + if (this.position.line.isValid() and this.position.column.isValid()) { if (this.enable_color) { try std.fmt.format( writer, - // : comptime Output.prettyFmt(":{d}:{d}", true), - .{ this.position.line + 1, this.position.column_start + 1 }, + .{ this.position.line.oneBased(), this.position.column.oneBased() }, ); } else { - try std.fmt.format(writer, ":{d}:{d}", .{ this.position.line + 1, this.position.column_start + 1 }); + try std.fmt.format(writer, ":{d}:{d}", .{ + this.position.line.oneBased(), + this.position.column.oneBased(), + }); } - } else if (this.position.line > -1) { + } else if (this.position.line.isValid()) { if (this.enable_color) { try std.fmt.format( writer, comptime Output.prettyFmt(":{d}", true), .{ - this.position.line + 1, + this.position.line.oneBased(), }, ); } else { try std.fmt.format(writer, ":{d}", .{ - this.position.line + 1, + this.position.line.oneBased(), }); } } @@ -716,28 +709,32 @@ pub const ZigStackFrame = extern struct { }; pub const ZigStackFramePosition = extern struct { - source_offset: i32, - line: i32, - line_start: i32, - line_stop: i32, - column_start: i32, - column_stop: i32, - expression_start: i32, - expression_stop: i32, + line: bun.Ordinal, + column: bun.Ordinal, + /// -1 if not present + line_start_byte: c_int, pub const Invalid = ZigStackFramePosition{ - .source_offset = -1, - .line = -1, - .line_start = -1, - .line_stop = -1, - .column_start = -1, - .column_stop = -1, - .expression_start = -1, - .expression_stop = -1, + .line = .invalid, + .column = .invalid, + .line_start_byte = -1, }; + pub fn isInvalid(this: *const ZigStackFramePosition) bool { return std.mem.eql(u8, std.mem.asBytes(this), std.mem.asBytes(&Invalid)); } + + pub fn decode(reader: anytype) !@This() { + return .{ + .line = bun.Ordinal.fromZeroBased(try reader.readValue(i32)), + .column = bun.Ordinal.fromZeroBased(try reader.readValue(i32)), + }; + } + + pub fn encode(this: *const @This(), writer: anytype) anyerror!void { + try writer.writeInt(this.line.zeroBased()); + try writer.writeInt(this.column.zeroBased()); + } }; pub const ZigException = extern struct { diff --git a/src/bun.js/bindings/headers-handwritten.h b/src/bun.js/bindings/headers-handwritten.h index acd26dd1d2..e8068957f4 100644 --- a/src/bun.js/bindings/headers-handwritten.h +++ b/src/bun.js/bindings/headers-handwritten.h @@ -1,4 +1,6 @@ #pragma once +#include "wtf/Compiler.h" +#include "wtf/text/OrdinalNumber.h" #ifndef HEADERS_HANDWRITTEN #define HEADERS_HANDWRITTEN typedef uint16_t ZigErrorCode; @@ -138,14 +140,18 @@ const ZigStackFrameCode ZigStackFrameCodeWasm = 5; const ZigStackFrameCode ZigStackFrameCodeConstructor = 6; typedef struct ZigStackFramePosition { - int32_t source_offset; - int32_t line; - int32_t line_start; - int32_t line_stop; - int32_t column_start; - int32_t column_stop; - int32_t expression_start; - int32_t expression_stop; + int32_t line_zero_based; + int32_t column_zero_based; + int32_t byte_position; + + ALWAYS_INLINE WTF::OrdinalNumber column() + { + return OrdinalNumber::fromZeroBasedInt(this->column_zero_based); + } + ALWAYS_INLINE WTF::OrdinalNumber line() + { + return OrdinalNumber::fromZeroBasedInt(this->line_zero_based); + } } ZigStackFramePosition; typedef struct ZigStackFrame { @@ -158,7 +164,7 @@ typedef struct ZigStackFrame { typedef struct ZigStackTrace { BunString* source_lines_ptr; - int32_t* source_lines_numbers; + OrdinalNumber* source_lines_numbers; uint8_t source_lines_len; uint8_t source_lines_to_collect; ZigStackFrame* frames_ptr; diff --git a/src/bun.js/javascript.zig b/src/bun.js/javascript.zig index 1b8f11c4e3..e87864c85e 100644 --- a/src/bun.js/javascript.zig +++ b/src/bun.js/javascript.zig @@ -97,6 +97,8 @@ const BuildMessage = JSC.BuildMessage; const ResolveMessage = JSC.ResolveMessage; const Async = bun.Async; +const Ordinal = bun.Ordinal; + pub const OpaqueCallback = *const fn (current: ?*anyopaque) callconv(.C) void; pub fn OpaqueWrap(comptime Context: type, comptime Function: fn (this: *Context) void) OpaqueCallback { return struct { @@ -2834,8 +2836,8 @@ pub const VirtualMachine = struct { if (this.source_mappings.resolveMapping( sourceURL.slice(), - @max(frame.position.line, 0), - @max(frame.position.column_start, 0), + @max(frame.position.line.zeroBased(), 0), + @max(frame.position.column.zeroBased(), 0), .no_source_contents, )) |lookup| { if (lookup.displaySourceURLIfNeeded(sourceURL.slice())) |source_url| { @@ -2843,8 +2845,8 @@ pub const VirtualMachine = struct { frame.source_url = source_url; } const mapping = lookup.mapping; - frame.position.line = mapping.original.lines; - frame.position.column_start = mapping.original.columns; + frame.position.line = Ordinal.fromZeroBased(mapping.original.lines); + frame.position.column = Ordinal.fromZeroBased(mapping.original.columns); frame.remapped = true; } else { // we don't want it to be remapped again @@ -2944,8 +2946,8 @@ pub const VirtualMachine = struct { .mapping = .{ .generated = .{}, .original = .{ - .lines = @max(top.position.line, 0), - .columns = @max(top.position.column_start, 0), + .lines = @max(top.position.line.zeroBased(), 0), + .columns = @max(top.position.column.zeroBased(), 0), }, .source_index = 0, }, @@ -2956,8 +2958,8 @@ pub const VirtualMachine = struct { else this.source_mappings.resolveMapping( top_source_url.slice(), - @max(top.position.line, 0), - @max(top.position.column_start, 0), + @max(top.position.line.zeroBased(), 0), + @max(top.position.column.zeroBased(), 0), .source_contents, ); @@ -2986,18 +2988,13 @@ pub const VirtualMachine = struct { }; source_code_slice.* = code; - top.position.line = mapping.original.lines; - top.position.line_start = mapping.original.lines; - top.position.line_stop = mapping.original.lines + 1; - top.position.column_start = mapping.original.columns; - top.position.column_stop = mapping.original.columns + 1; + top.position.line = Ordinal.fromZeroBased(mapping.original.lines); + top.position.column = Ordinal.fromZeroBased(mapping.original.columns); + exception.remapped = true; top.remapped = true; - // This expression range is no longer accurate - top.position.expression_start = mapping.original.columns; - top.position.expression_stop = mapping.original.columns + 1; - const last_line = @max(top.position.line, 0); + const last_line = @max(top.position.line.zeroBased(), 0); if (strings.getLinesInText( code.slice(), @intCast(last_line), @@ -3020,13 +3017,6 @@ pub const VirtualMachine = struct { } exception.stack.source_lines_len = @as(u8, @truncate(lines.len)); - - top.position.column_stop = @as(i32, @intCast(source_lines[lines.len - 1].length())); - top.position.line_stop = top.position.column_stop; - - // This expression range is no longer accurate - top.position.expression_start = mapping.original.columns; - top.position.expression_stop = top.position.column_stop; } } @@ -3037,8 +3027,8 @@ pub const VirtualMachine = struct { defer source_url.deinit(); if (this.source_mappings.resolveMapping( source_url.slice(), - @max(frame.position.line, 0), - @max(frame.position.column_start, 0), + @max(frame.position.line.zeroBased(), 0), + @max(frame.position.column.zeroBased(), 0), .no_source_contents, )) |lookup| { if (lookup.displaySourceURLIfNeeded(source_url.slice())) |src| { @@ -3046,9 +3036,9 @@ pub const VirtualMachine = struct { frame.source_url = src; } const mapping = lookup.mapping; - frame.position.line = mapping.original.lines; frame.remapped = true; - frame.position.column_start = mapping.original.columns; + frame.position.line = Ordinal.fromZeroBased(mapping.original.lines); + frame.position.column = Ordinal.fromZeroBased(mapping.original.columns); } } } @@ -3197,8 +3187,8 @@ pub const VirtualMachine = struct { .{ display_line, bun.fmt.fmtJavaScript(clamped, allow_ansi_color) }, ); - if (clamped.len < max_line_length_with_divot or top.position.column_start > max_line_length_with_divot) { - const indent = max_line_number_pad + " | ".len + @as(u64, @intCast(top.position.column_start)); + if (clamped.len < max_line_length_with_divot or top.position.column.zeroBased() > max_line_length_with_divot) { + const indent = max_line_number_pad + " | ".len + @as(u64, @intCast(top.position.column.zeroBased())); try writer.writeByteNTimes(' ', indent); try writer.print(comptime Output.prettyFmt( @@ -3380,8 +3370,8 @@ pub const VirtualMachine = struct { const file = bun.path.relative(dir, source_url.slice()); writer.print("\n::error file={s},line={d},col={d},title=", .{ file, - frame.position.line_start + 1, - frame.position.column_start, + frame.position.line.oneBased(), + frame.position.column.oneBased(), }) catch {}; has_location = true; } diff --git a/src/bun.js/module_loader.zig b/src/bun.js/module_loader.zig index 7505a8db9e..1c16809e45 100644 --- a/src/bun.js/module_loader.zig +++ b/src/bun.js/module_loader.zig @@ -234,6 +234,7 @@ fn dumpSourceStringFailiable(vm: *VirtualMachine, specifier: string, written: [] js_printer.formatJSONStringUTF8(source_file), mappings.formatVLQs(), }); + try bufw.flush(); } } else { dir.writeFile(std.fs.path.basename(specifier), written) catch return; diff --git a/src/bun.zig b/src/bun.zig index eae4e74c0e..214f69b033 100644 --- a/src/bun.zig +++ b/src/bun.zig @@ -3458,3 +3458,47 @@ pub const timespec = extern struct { return now().addMs(interval); } }; + +/// An abstract number of element in a sequence. The sequence has a first element. +/// This type should be used instead of integer because 2 contradicting traditions can +/// call a first element '0' or '1' which makes integer type ambiguous. +pub fn OrdinalT(comptime Int: type) type { + return enum(Int) { + invalid = switch (@typeInfo(Int).Int.signedness) { + .unsigned => std.math.maxInt(Int), + .signed => -1, + }, + start = 0, + _, + + pub fn fromZeroBased(int: Int) @This() { + assert(int >= 0); + assert(int != std.math.maxInt(Int)); + return @enumFromInt(int); + } + + pub fn fromOneBased(int: Int) @This() { + assert(int > 0); + return @enumFromInt(int - 1); + } + + pub fn zeroBased(ord: @This()) Int { + return @intFromEnum(ord); + } + + pub fn oneBased(ord: @This()) Int { + return @intFromEnum(ord) + 1; + } + + pub fn add(ord: @This(), inc: Int) @This() { + return fromZeroBased(ord.zeroBased() + inc); + } + + pub fn isValid(ord: @This()) bool { + return ord.zeroBased() >= 0; + } + }; +} + +/// ABI-equivalent of WTF::OrdinalNumber +pub const Ordinal = OrdinalT(c_int); diff --git a/test/js/node/v8/error-prepare-stack-default-fixture.js b/test/js/node/v8/error-prepare-stack-default-fixture.js index 206ad15e3a..2586758595 100644 --- a/test/js/node/v8/error-prepare-stack-default-fixture.js +++ b/test/js/node/v8/error-prepare-stack-default-fixture.js @@ -15,9 +15,10 @@ const err2 = new Error(); Error.captureStackTrace(err2); const stack2 = err2.stack; -const stackIgnoringLineAndColumn = stack2.replaceAll(":16:2", "N"); -const stack2IgnoringLineAndColumn = stack.replaceAll(":11:2", "N"); +const stackIgnoringLineAndColumn = stack.replaceAll(":10:24", "N"); +const stack2IgnoringLineAndColumn = stack2.replaceAll(":15:24", "N"); if (stackIgnoringLineAndColumn !== stack2IgnoringLineAndColumn) { - console.log(stackIgnoringLineAndColumn, stack2IgnoringLineAndColumn); + console.log(stackIgnoringLineAndColumn); + console.log(stack2IgnoringLineAndColumn); throw new Error("Stacks are different"); }