After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 697309 - Fix some compile warnings with js17
Fix some compile warnings with js17
Status: RESOLVED OBSOLETE
Product: gjs
Classification: Bindings
Component: general
unspecified
Other All
: Normal normal
: ---
Assigned To: gjs-maint
gjs-maint
Depends on:
Blocks:
 
 
Reported: 2013-04-05 06:47 UTC by Jasper St. Pierre (not reading bugmail)
Modified: 2013-05-09 13:57 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
gerror: Fix a broken signature (733 bytes, patch)
2013-04-05 06:47 UTC, Jasper St. Pierre (not reading bugmail)
committed Details | Review
jsapi-util: Remove unused variable (667 bytes, patch)
2013-04-05 06:47 UTC, Jasper St. Pierre (not reading bugmail)
committed Details | Review
compat: Fix unused method warnings (803 bytes, patch)
2013-04-05 06:47 UTC, Jasper St. Pierre (not reading bugmail)
committed Details | Review
context: Fix the signature of the GC callback (1.46 KB, patch)
2013-04-05 06:47 UTC, Jasper St. Pierre (not reading bugmail)
committed Details | Review
profiler: Fix some profiler warnings (1.98 KB, patch)
2013-04-05 06:47 UTC, Jasper St. Pierre (not reading bugmail)
committed Details | Review
Fix a lot of property op and class init warnings (23.46 KB, patch)
2013-04-05 06:47 UTC, Jasper St. Pierre (not reading bugmail)
committed Details | Review
Add a wrapper for finalize (9.50 KB, patch)
2013-04-05 06:47 UTC, Jasper St. Pierre (not reading bugmail)
reviewed Details | Review

Description Jasper St. Pierre (not reading bugmail) 2013-04-05 06:47:09 UTC
See patches. Doesn't fix all warnings.
Comment 1 Jasper St. Pierre (not reading bugmail) 2013-04-05 06:47:13 UTC
Created attachment 240692 [details] [review]
gerror: Fix a broken signature
Comment 2 Jasper St. Pierre (not reading bugmail) 2013-04-05 06:47:15 UTC
Created attachment 240693 [details] [review]
jsapi-util: Remove unused variable
Comment 3 Jasper St. Pierre (not reading bugmail) 2013-04-05 06:47:18 UTC
Created attachment 240694 [details] [review]
compat: Fix unused method warnings
Comment 4 Jasper St. Pierre (not reading bugmail) 2013-04-05 06:47:22 UTC
Created attachment 240695 [details] [review]
context: Fix the signature of the GC callback
Comment 5 Jasper St. Pierre (not reading bugmail) 2013-04-05 06:47:25 UTC
Created attachment 240696 [details] [review]
profiler: Fix some profiler warnings
Comment 6 Jasper St. Pierre (not reading bugmail) 2013-04-05 06:47:28 UTC
Created attachment 240697 [details] [review]
Fix a lot of property op and class init warnings
Comment 7 Jasper St. Pierre (not reading bugmail) 2013-04-05 06:47:31 UTC
Created attachment 240698 [details] [review]
Add a wrapper for finalize

This wraps JSFreeOp and makes a context for the finalizer run.
Comment 8 Colin Walters 2013-04-05 22:58:20 UTC
Review of attachment 240692 [details] [review]:

Yep.
Comment 9 Colin Walters 2013-04-05 22:58:42 UTC
Review of attachment 240693 [details] [review]:

I'd be fine with just committing unused variable fixes.
Comment 10 Colin Walters 2013-04-05 22:59:06 UTC
Review of attachment 240694 [details] [review]:

Right.
Comment 11 Colin Walters 2013-04-05 23:00:07 UTC
Review of attachment 240695 [details] [review]:

Yep.  (But note we'd eventually like to have control over the GC, but that's something we have to fix upstream)
Comment 12 Colin Walters 2013-04-05 23:00:39 UTC
Review of attachment 240696 [details] [review]:

Hooray for deleting dead code.
Comment 13 Colin Walters 2013-04-05 23:05:11 UTC
Review of attachment 240697 [details] [review]:

Interesting.  Yes, this looks right.

::: gjs/jsapi-util.c
@@ +84,3 @@
 
+static void
+finalize_stub(JSFreeOp *fop, JSObject *global) {}

Can you put the parens each on a new line here?  It just helps me visually scan.
Comment 14 Colin Walters 2013-04-05 23:09:29 UTC
Review of attachment 240698 [details] [review]:

Is this really necessary?  Or only if we invoke JSAPI from the finalizer?

Looking at the js source, there's e.g. xmlfilter_finalize() which doesn't appear to create a context.  Hmm...feels like this one needs more investigation.
Comment 15 Jasper St. Pierre (not reading bugmail) 2013-04-05 23:14:35 UTC
Review of attachment 240698 [details] [review]:

It was simply the easiest way to get a context from the runtime provided. Should I instead just use JS_GetRuntimeData to get the same context?

I feel that since the finalizer will be called on the GC thread, it makes more sense to create a context rather than use the main thread only one.
Comment 16 Jasper St. Pierre (not reading bugmail) 2013-04-07 18:25:55 UTC
Attachment 240692 [details] pushed as 5f54f55 - gerror: Fix a broken signature
Attachment 240693 [details] pushed as aecd778 - jsapi-util: Remove unused variable
Attachment 240694 [details] pushed as 96388ed - compat: Fix unused method warnings
Attachment 240695 [details] pushed as f503b31 - context: Fix the signature of the GC callback
Attachment 240696 [details] pushed as 406b0b3 - profiler: Fix some profiler warnings
Attachment 240697 [details] pushed as 0bd8418 - Fix a lot of property op and class init warnings