JavaScriptCore API should support type checking for Array and Date
Created attachment 249953 [details] Patch
Attachment 249953 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/API/JSValueRef.h:139: The parameter name "value" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/JavaScriptCore/API/JSValueRef.h:148: The parameter name "value" adds no information, so it should be removed. [readability/parameter_name] [5] Total errors found: 2 in 7 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 249953 [details] Patch These probably need availability macros.
Comment on attachment 249953 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=249953&action=review Should make these properties as suggested by the API experts here at Apple. > Source/JavaScriptCore/API/JSValueRef.cpp:179 > + JSValue jsValue = toJS(exec, value); > + return jsValue.inherits(JSArray::info()); I’d like this better as a one-liner. > Source/JavaScriptCore/API/JSValueRef.cpp:192 > + JSValue jsValue = toJS(exec, value); > + return jsValue.inherits(DateInstance::info()); Ditto. > Source/JavaScriptCore/API/tests/testapi.c:1416 > + JSStringRelease(str); Nice to fix the leak. I think the code below also leaks an exception object.
> View in context: > https://bugs.webkit.org/attachment.cgi?id=249953&action=review > > Should make these properties as suggested by the API experts here at Apple. I'll follow up with a patch to do that for all of these related is* methods, once the discussion is settled. > > Source/JavaScriptCore/API/JSValueRef.cpp:179 > > + JSValue jsValue = toJS(exec, value); > > + return jsValue.inherits(JSArray::info()); > > I’d like this better as a one-liner. > > > Source/JavaScriptCore/API/JSValueRef.cpp:192 > > + JSValue jsValue = toJS(exec, value); > > + return jsValue.inherits(DateInstance::info()); > > Ditto. OK. > Nice to fix the leak. I think the code below also leaks an exception object. The exception local variable is a JSValueRef, so it's garbage collected.
Committed r182297: <http://trac.webkit.org/changeset/182297>
Comment on attachment 249953 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=249953&action=review > Source/JavaScriptCore/API/JSValue.h:422 > +- (BOOL)isArray; Should these have NS_AVAILABLE macros? > Source/JavaScriptCore/API/JSValueRef.h:139 > +JS_EXPORT bool JSValueIsArray(JSContextRef ctx, JSValueRef value); Should these have CF_AVAILABLE macros?
(In reply to comment #7) > Comment on attachment 249953 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=249953&action=review > > > Source/JavaScriptCore/API/JSValue.h:422 > > +- (BOOL)isArray; > > Should these have NS_AVAILABLE macros? > > > Source/JavaScriptCore/API/JSValueRef.h:139 > > +JS_EXPORT bool JSValueIsArray(JSContextRef ctx, JSValueRef value); > > Should these have CF_AVAILABLE macros? Ahh, I see when this landed it had them!