fix the rest of node:buffer #15722
fix the rest of node:buffer #15722
Conversation
|
|
||
| const toPrimitive = $tryGetByIdWithWellKnownSymbol(value, "toPrimitive"); |
There was a problem hiding this comment.
You want to do this instead of value + ""?
| JSC::EncodedJSValue V::validateInteger(JSC::ThrowScope& scope, JSC::JSGlobalObject* globalObject, JSC::JSValue value, const WTF::String& name, JSC::JSValue min, JSC::JSValue max) | ||
| { | ||
| if (!value.isNumber()) return Bun::ERR::INVALID_ARG_TYPE(scope, globalObject, name, "number"_s, value); | ||
| if (min.isUndefined()) min = jsDoubleNumber(JSC::minSafeInteger()); |
There was a problem hiding this comment.
When these values are coming from C++ (not JavaScript), we shouldn't be converting from C++ -> JS types -> back.
Especially when it's compile-time known.
|
|
||
| if (!JSC::isArray(globalObject, value)) return Bun::ERR::INVALID_ARG_TYPE(scope, globalObject, name, "Array"_s, value); | ||
|
|
||
| auto length = value.get(globalObject, Identifier::fromString(vm, "length"_s)); |
There was a problem hiding this comment.
This getter call, and the length coercion below is unnecessary and likely will cause bugs.
Plus, the identifier "length" is in the special list of already available properties in the JSC::VM. vm.propertyNames.length.
|
|
||
| if (minLength.isUndefined()) minLength = jsNumber(0); | ||
|
|
||
| if (!JSC::isArray(globalObject, value)) return Bun::ERR::INVALID_ARG_TYPE(scope, globalObject, name, "Array"_s, value); |
There was a problem hiding this comment.
Instead of using JSC::isArray, and then otherwise treating it as a normal object, we can cast it to a JSArray* and then call length on that JSArray. It is stored as an unsigned integer.
| if (!JSC::isArray(globalObject, value)) return Bun::ERR::INVALID_ARG_TYPE(scope, globalObject, name, "Array"_s, value); | |
| auto *array = jsDynamicCast<JSC::JSArray*>(value); | |
| if (!array) | |
| return Bun::ERR::INVALID_ARG_TYPE(scope, globalObject, name, "Array"_s, value); |
|
This PR makes Buffer-related methods 3x - 5x slower. We are not merging this until that performance regression is fixed. 
|
| @@ -201,14 +204,14 @@ WTF::String JSValueToStringSafe(JSC::JSGlobalObject* globalObject, JSValue arg) | |||
| auto name = JSC::getCalculatedDisplayName(vm, cell->getObject()); | |||
| if (catchScope.exception()) { | |||
| catchScope.clearException(); | |||
| name = "Function"_s; | |||
| name = ""_s; | |||
There was a problem hiding this comment.
| name = ""_s; | |
| return "[Function: (anonymous)]"_s; |
There was a problem hiding this comment.
I assuming that is fine to ignore this exception
missing some cosmetic tests but all the functionality is there now