-
Notifications
You must be signed in to change notification settings - Fork 5k
node:util: add diff() (Myers diff port) #39749
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -24,6 +24,113 @@ const ObjectGetOwnPropertyNames = Object.getOwnPropertyNames; | |
| const { uncurryThis, SafeMap } = require("internal/primordials"); | ||
| const RegExpPrototypeExec = uncurryThis(RegExp.prototype.exec); | ||
|
|
||
| const ArrayPrototypePush = Array.prototype.push; | ||
| const ArrayPrototypeReverse = Array.prototype.reverse; | ||
|
|
||
| function areLinesEqual(actual, expected) { | ||
| return actual === expected; | ||
| } | ||
|
|
||
| // Myers diff (Node's internal/assert/myers_diff). Returns an array of | ||
| // [operation, value] pairs: -1 delete, 0 no-op, 1 insert. | ||
| function myersDiff(actual, expected) { | ||
| const actualLength = actual.length; | ||
| const expectedLength = expected.length; | ||
| const max = actualLength + expectedLength; | ||
| const v = new Int32Array(2 * max + 1); | ||
|
Comment on lines
+39
to
+40
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -eu
printf '%s\n' '--- util.ts structure and diff implementation ---'
ast-grep outline src/js/node/util.ts --match diff --view expanded || true
printf '%s\n' '--- relevant source ---'
sed -n '1,155p' src/js/node/util.ts
printf '%s\n' '--- size/error handling references ---'
rg -n 'ERR_OUT_OF_RANGE|actualLength|expectedLength|myers|Int32Array' src/js/node/util.ts src/js -g '*.ts' -g '*.js' | head -200Repository: oven-sh/bun Length of output: 14578 🏁 Script executed: #!/bin/bash
set -eu
python3 - <<'PY'
import urllib.request
urls = [
"https://raw.githubusercontent.com/nodejs/node/v24.11.0/lib/internal/assert/myers_diff.js",
"https://raw.githubusercontent.com/nodejs/node/v24.11.0/lib/internal/assert/assertion_error.js",
]
for url in urls:
print(f"--- {url} ---")
try:
print(urllib.request.urlopen(url, timeout=10).read().decode())
except Exception as exc:
print(f"FETCH_ERROR: {exc}")
PY
printf '%s\n' '--- repository history summary for the relevant file ---'
git log --oneline -8 -- src/js/node/util.ts
printf '%s\n' '--- local typed-array size assumptions and tests ---'
rg -n '2 \*\* 31|MAX_LENGTH|Int32Array|ERR_OUT_OF_RANGE' test tests src/js/internal/assert src/js/node -g '*.js' -g '*.ts' -g '*.mjs' 2>/dev/null | head -250Repository: oven-sh/bun Length of output: 27253 🏁 Script executed: #!/bin/bash
set -eu
printf '%s\n' '--- Node v24.11 Myers implementation ---'
curl -ksSf https://raw.githubusercontent.com/nodejs/node/v24.11.0/lib/internal/assert/myers_diff.js | sed -n '1,180p'
printf '%s\n' '--- commit introducing the Bun port ---'
git show --format=fuller --stat --oneline ff029598 -- src/js/node/util.ts
git show --format= --no-ext-diff ff029598 -- src/js/node/util.ts | sed -n '1,180p'
printf '%s\n' '--- standalone typed-array constructor boundary probe ---'
node - <<'JS'
const candidates = [2 ** 31 - 1, 2 ** 31, 2 ** 32 - 1, 2 ** 32];
for (const length of candidates) {
try {
// Use only the constructor's argument validation; avoid allocating large backing stores.
const result = new Int32Array({ length });
console.log(length, result.length);
} catch (error) {
console.log(length, error.name, error.code, error.message.split('\n')[0]);
}
}
JSRepository: oven-sh/bun Length of output: 9671 Reject oversized Myers diff inputs. If 🤖 Prompt for AI AgentsSource: Coding guidelines |
||
| const trace = []; | ||
|
|
||
| for (let diffLevel = 0; diffLevel <= max; diffLevel++) { | ||
| ArrayPrototypePush.call(trace, new Int32Array(v)); | ||
|
|
||
| for (let diagonalIndex = -diffLevel; diagonalIndex <= diffLevel; diagonalIndex += 2) { | ||
| const offset = diagonalIndex + max; | ||
| const previousOffset = v[offset - 1]; | ||
| const nextOffset = v[offset + 1]; | ||
| let x = | ||
| diagonalIndex === -diffLevel || (diagonalIndex !== diffLevel && previousOffset < nextOffset) | ||
| ? nextOffset | ||
| : previousOffset + 1; | ||
| let y = x - diagonalIndex; | ||
|
|
||
| while (x < actualLength && y < expectedLength && areLinesEqual(actual[x], expected[y])) { | ||
| x++; | ||
| y++; | ||
| } | ||
|
|
||
| v[offset] = x; | ||
|
|
||
| if (x >= actualLength && y >= expectedLength) { | ||
| return backtrack(trace, actual, expected); | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| function backtrack(trace, actual, expected) { | ||
| const actualLength = actual.length; | ||
| const expectedLength = expected.length; | ||
| const max = actualLength + expectedLength; | ||
|
|
||
| let x = actualLength; | ||
| let y = expectedLength; | ||
| const result = []; | ||
|
|
||
| for (let diffLevel = trace.length - 1; diffLevel >= 0; diffLevel--) { | ||
| const v = trace[diffLevel]; | ||
| const diagonalIndex = x - y; | ||
| const offset = diagonalIndex + max; | ||
|
|
||
| let prevDiagonalIndex; | ||
| if ( | ||
| diagonalIndex === -diffLevel || | ||
| (diagonalIndex !== diffLevel && v[offset - 1] < v[offset + 1]) | ||
| ) { | ||
| prevDiagonalIndex = diagonalIndex + 1; | ||
| } else { | ||
| prevDiagonalIndex = diagonalIndex - 1; | ||
| } | ||
|
|
||
| const prevX = v[prevDiagonalIndex + max]; | ||
| const prevY = prevX - prevDiagonalIndex; | ||
|
|
||
| while (x > prevX && y > prevY) { | ||
| ArrayPrototypePush.call(result, [0, actual[x - 1]]); | ||
| x--; | ||
| y--; | ||
| } | ||
|
|
||
| if (diffLevel > 0) { | ||
| if (x > prevX) { | ||
| ArrayPrototypePush.call(result, [1, actual[--x]]); | ||
| } else { | ||
| ArrayPrototypePush.call(result, [-1, expected[--y]]); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| return result; | ||
| } | ||
|
|
||
| function validateDiffInput(value, name) { | ||
| if (Array.isArray(value)) { | ||
| for (let i = 0; i < value.length; i++) { | ||
| validateString(value[i], `${name}[${i}]`); | ||
| } | ||
| return; | ||
| } | ||
| validateString(value, name); | ||
| } | ||
|
|
||
| function diff(actual, expected) { | ||
| if (actual === expected) { | ||
| return []; | ||
| } | ||
| validateDiffInput(actual, "actual"); | ||
| validateDiffInput(expected, "expected"); | ||
| return ArrayPrototypeReverse.call(myersDiff(actual, expected)); | ||
| } | ||
|
|
||
| var cjs_exports; | ||
|
|
||
| function isBuffer(value) { | ||
|
|
@@ -698,6 +805,7 @@ cjs_exports = { | |
| debug: debuglog, | ||
| debuglog, | ||
| deprecate, | ||
| diff, | ||
| format, | ||
| styleText, | ||
| formatWithOptions, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4248,6 +4248,25 @@ JSC_DEFINE_HOST_FUNCTION(Process_stubEmptyFunction, (JSGlobalObject * globalObje | |
| return JSValue::encode(jsUndefined()); | ||
| } | ||
|
|
||
| JSC_DEFINE_CUSTOM_GETTER(processSourceMapsEnabled, (JSC::JSGlobalObject * globalObject, JSC::EncodedJSValue thisValue, JSC::PropertyName)) | ||
| { | ||
| Zig::GlobalObject* globalObj = defaultGlobalObject(globalObject); | ||
| return JSValue::encode(jsBoolean(globalObj->processObject()->m_sourceMapsEnabled)); | ||
| } | ||
|
|
||
| JSC_DEFINE_CUSTOM_SETTER(setProcessSourceMapsEnabled, (JSC::JSGlobalObject * globalObject, JSC::EncodedJSValue thisValue, JSC::EncodedJSValue encodedValue, JSC::PropertyName propertyName)) | ||
| { | ||
| auto& vm = JSC::getVM(globalObject); | ||
| auto scope = DECLARE_THROW_SCOPE(vm); | ||
| JSValue value = JSValue::decode(encodedValue); | ||
| if (!value.isBoolean()) { | ||
| return Bun::ERR::INVALID_ARG_TYPE(scope, globalObject, "enabled"_s, "boolean"_s, value); | ||
| } | ||
| Zig::GlobalObject* globalObj = defaultGlobalObject(globalObject); | ||
| globalObj->processObject()->m_sourceMapsEnabled = value.toBoolean(globalObject); | ||
| return true; | ||
|
Comment on lines
+4257
to
+4267
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -eu
printf '%s\n' '--- target implementation ---'
sed -n '4225,4280p' src/jsc/bindings/BunProcess.cpp
printf '%s\n' '--- accessor registration ---'
sed -n '4895,4940p' src/jsc/bindings/BunProcess.cpp
printf '%s\n' '--- related source-map references ---'
rg -n -C 3 'sourceMapsEnabled|setSourceMapsEnabled|SourceMaps' src test | head -240
printf '%s\n' '--- repository review guidance ---'
if [ -f REVIEW.md ]; then sed -n '1,220p' REVIEW.md; fi
printf '%s\n' '--- Node v24.11.0 reference ---'
curl -LfsS https://raw.githubusercontent.com/nodejs/node/v24.11.0/lib/internal/bootstrap/node.js |
rg -n -C 8 'sourceMapsEnabled|setSourceMapsEnabled'Repository: oven-sh/bun Length of output: 50367 🏁 Script executed: #!/bin/bash
set -eu
printf '%s\n' '--- complete local source-map methods ---'
sed -n '4251,4292p' src/jsc/bindings/BunProcess.cpp
printf '%s\n' '--- local source-map test ---'
cat -n test/js/node/process/process-sourcemaps-enabled.test.ts
printf '%s\n' '--- Node v24.11.0 implementation ---'
curl -LfsS https://raw.githubusercontent.com/nodejs/node/v24.11.0/lib/internal/bootstrap/node.js |
sed -n '/sourceMapsEnabled/,+18p'
printf '%s\n' '--- Node v24.11.0 descriptor-related tests ---'
curl -LfsS https://raw.githubusercontent.com/nodejs/node/v24.11.0/test/parallel/test-process-setsourcemapsenabled.js |
sed -n '1,100p'Repository: oven-sh/bun Length of output: 3875 🏁 Script executed: #!/bin/bash
set -eu
python3 - <<'PY'
from pathlib import Path
import re
import urllib.request
local = Path("src/jsc/bindings/BunProcess.cpp").read_text()
test = Path("test/js/node/process/process-sourcemaps-enabled.test.ts").read_text()
node = urllib.request.urlopen(
"https://raw.githubusercontent.com/nodejs/node/v24.11.0/lib/internal/bootstrap/node.js",
timeout=10,
).read().decode()
assert "JSC_DEFINE_CUSTOM_SETTER(setProcessSourceMapsEnabled" in local
assert re.search(
r"sourceMapsEnabled\s+processSourceMapsEnabled\s+CustomAccessor",
local,
)
assert "process.setSourceMapsEnabled" not in test
assert re.search(
r"ObjectDefineProperty\(process,\s*'sourceMapsEnabled'.*?"
r"get\(\)\s*\{.*?\},\s*\}\s*\);",
node,
re.S,
)
descriptor = node[node.index("ObjectDefineProperty(process, 'sourceMapsEnabled'"):
node.index("process.setSourceMapsEnabled", node.index(
"ObjectDefineProperty(process, 'sourceMapsEnabled'"))]
assert "set()" not in descriptor
assert "set:" not in descriptor
print("local accessor has a setter; Node v24.11.0 descriptor has no setter; test lacks set assertion")
PY
if command -v node >/dev/null 2>&1; then
node - <<'JS'
const descriptor = Object.getOwnPropertyDescriptor(process, "sourceMapsEnabled");
console.log(JSON.stringify({
nodeVersion: process.version,
hasGetter: typeof descriptor?.get === "function",
hasSetter: descriptor?.set !== undefined,
}));
JS
fiRepository: oven-sh/bun Length of output: 2435 Keep Node v24.11.0 defines this property with a getter and no setter. Remove 🤖 Prompt for AI AgentsSource: MCP tools |
||
| } | ||
|
|
||
| JSC_DEFINE_HOST_FUNCTION(Process_setSourceMapsEnabled, (JSC::JSGlobalObject * lexicalGlobalObject, JSC::CallFrame* callFrame)) | ||
| { | ||
| Zig::GlobalObject* globalObject = defaultGlobalObject(lexicalGlobalObject); | ||
|
|
@@ -4901,6 +4920,7 @@ extern "C" void Process__emitErrorEvent(Zig::GlobalObject* global, EncodedJSValu | |
| send constructProcessSend PropertyCallback | ||
| setSourceMapsEnabled Process_setSourceMapsEnabled Function 1 | ||
| setUncaughtExceptionCaptureCallback Process_setUncaughtExceptionCaptureCallback Function 1 | ||
| sourceMapsEnabled processSourceMapsEnabled CustomAccessor | ||
| stderr constructStderr PropertyCallback | ||
| stdin constructStdin PropertyCallback | ||
| stdout constructStdout PropertyCallback | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
Repository: oven-sh/bun
Length of output: 20008
🏁 Script executed:
Repository: oven-sh/bun
Length of output: 45975
🏁 Script executed:
Repository: oven-sh/bun
Length of output: 42198
🏁 Script executed:
Repository: oven-sh/bun
Length of output: 11459
Use tamper-resistant Array intrinsics and
$call.Replace
Array.prototype.push,Array.prototype.reverse, andArray.isArraywith$Arrayintrinsics. Invoke the captured methods with.$callinstead of.call. This prevents mutable globals from changingutil.diffvalidation or result construction.🤖 Prompt for AI Agents
Source: Coding guidelines