static analysis failure, with NS_DECL_NSIXPCSCRIPTABLE causing "error: The fully qualified types are preferred over the shorthand typedefs for JS::Handle/JS::Rooted types outside SpiderMonkey."
Categories
(Core :: XPConnect, defect)
Tracking
()
Tracking | Status | |
---|---|---|
firefox108 | --- | fixed |
People
(Reporter: dholbert, Assigned: dholbert)
References
Details
Attachments
(2 files)
I just tried building with static analysis enabled for the first time in a while. My local build uses distributed sccache which does preprocessor-expansion separately from compilation, for what it's worth.
And I'm hitting this build error:
0:13.82 In file included from Unified_cpp_storage1.cpp:29:
0:13.82 In file included from /scratch/work/builds/mozilla-central/mozilla/storage/mozStorageStatement.cpp:19:
0:13.82 /scratch/work/builds/mozilla-central/mozilla/storage/mozStorageStatementJSHelper.h:23:391: error: The fully qualified types are preferred over the shorthand typedefs for JS::Handle/JS::Rooted types outside SpiderMonkey.
0:13.82 virtual nsresult GetClassName(nsACString& aClassName) override; virtual uint32_t GetScriptableFlags(void) override; virtual const JSClass * GetJSClass(void) override; virtual nsresult PreCreate(nsISupports *nativeObj, JSContext * cx, JSObject * globalObj, JSObject * * parentObj) override; virtual nsresult NewEnumerate(nsIXPConnectWrappedNative *wrapper, JSContext * cx, JSObject * obj, JS::MutableHandleIdVector properties, bool enumerableOnly, bool *_retval) override; virtual nsresult Resolve(nsIXPConnectWrappedNative *wrapper, JSContext * cx, JSObject * obj, jsid id, bool *resolvedp, bool *_retval) override; virtual nsresult Finalize(nsIXPConnectWrappedNative *wrapper, JS::GCContext * gcx, JSObject * obj) override; virtual nsresult Call(nsIXPConnectWrappedNative *wrapper, JSContext * cx, JSObject * obj, const JS::CallArgs & args, bool *_retval) override; virtual nsresult Construct(nsIXPConnectWrappedNative *wrapper, JSContext * cx, JSObject * obj, const JS::CallArgs & args, bool *_retval) override; virtual nsresult HasInstance(nsIXPConnectWrappedNative *wrapper, JSContext * cx, JSObject * obj, JS::Handle<JS::Value> val, bool *bp, bool *_retval) override;
0:13.82 ^~~~~~~~~~~~~~~~~~~~~~~~~
0:13.82 JS::MutableHandleVector<JS::PropertyKey>
The compile error points to the last line here, the NS_DECL_NSIXPCSCRIPTABLE
macro:
https://searchfox.org/mozilla-central/rev/fe5c9c39a879b07d5b629257f63d825c3c8cd0ed/storage/mozStorageStatementJSHelper.h#20-23
class StatementJSHelper : public nsIXPCScriptable {
public:
NS_DECL_ISUPPORTS
NS_DECL_NSIXPCSCRIPTABLE
And in particular it seems to be complaining about an arg for the [EDIT: actually it's the NewEnumerate API, I skimmed the single-line API declaration too quickly], which is declared (in the expanded macro) as GetClassName
APIJS::MutableHandleIdVector properties
and static analysis warning is saying it should instead be declared with type JS::MutableHandleVector<JS::PropertyKey>
I guess.
Kagami, looks like this was a linter that you added -- would you mind taking a look? (Presumably we need to either update an allow-list somewhere, or fix the definition of NS_DECL_NSIXPCSCRIPTABLE
which would mean a tweak in XPIDL or something like that.)
Assignee | ||
Comment 1•2 years ago
|
||
Here's a manually-pruned section of the error message that's pointing to the warned-about arg:
virtual nsresult NewEnumerate([...], JSObject * obj, JS::MutableHandleIdVector properties,
^~~~~~~~~~~~~~~~~~~~~~~~~
JS::MutableHandleVector<JS::PropertyKey>
Assignee | ||
Comment 2•2 years ago
|
||
That seems to correspond to this chunk of IDL:
https://searchfox.org/mozilla-central/rev/fe5c9c39a879b07d5b629257f63d825c3c8cd0ed/js/xpconnect/idl/nsIXPCScriptable.idl#75-77
boolean newEnumerate(in nsIXPConnectWrappedNative wrapper,
in JSContextPtr cx, in JSObjectPtr obj,
in JSMutableHandleIdVector properties,
...with the type alias defined further up in that file at line 31:
https://searchfox.org/mozilla-central/rev/fe5c9c39a879b07d5b629257f63d825c3c8cd0ed/js/xpconnect/idl/nsIXPCScriptable.idl#31
native JSMutableHandleIdVector(JS::MutableHandleIdVector);
Maybe line 31 just needs to change to reference JS::MutableHandleVector<JS::PropertyKey>
...?
Assignee | ||
Comment 3•2 years ago
|
||
Yup, that seems to do it.
Assignee | ||
Updated•2 years ago
|
Assignee | ||
Comment 4•2 years ago
|
||
This is a non-functional change (just whitespace cleanup).
Assignee | ||
Comment 5•2 years ago
|
||
This patch doesn't change behavior; it's just expanding an abbreviated typename.
Without this patch, my local static-analysis build fails with the following,
fore.g. mozStorageStatementJSHelper.h (which has a class that implements
nsIXPCScriptable and is outside of spidermonkey):
"error: The fully qualified types are preferred over the shorthand typedefs for
JS::Handle/JS::Rooted types outside SpiderMonkey."
This patch's type-expansion seems to appease this static analysis rule.
Before this patch, the type here was JS::MutableHandleIdVector
which is a
typedef for MutableHandle<StackGCVector<JS::PropertyKey>>
as defined here:
https://searchfox.org/mozilla-central/rev/fe5c9c39a879b07d5b629257f63d825c3c8cd0ed/js/public/TypeDecls.h#98
After this patch, the type here is JS::MutableHandleVector<JS::PropertyKey>
(which I took from the static analysis error message's suggestion). That
expands to the same full-qualified type, since MutableHandleVector<T> is an
alias for MutableHandle<StackGCVector<T>> as defined here:
https://searchfox.org/mozilla-central/rev/fe5c9c39a879b07d5b629257f63d825c3c8cd0ed/js/public/TypeDecls.h#128
Depends on D160689
Comment 7•2 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/af0f176408d1
https://hg.mozilla.org/mozilla-central/rev/e10250770801
Description
•