Closed
Bug 723806
Opened 9 years ago
Closed 9 years ago
JS CTypes prints function types incorrectly
Categories
(Core :: js-ctypes, defect)
Core
js-ctypes
Tracking
()
RESOLVED
FIXED
mozilla13
People
(Reporter: jimb, Assigned: jimb)
Details
Attachments
(1 file, 2 obsolete files)
CTypes prints function types incorrectly. For example: js> ctypes.FunctionType(ctypes.default_abi, ctypes.int).toString() "type int()()" js> It should return "type int()".
Attachment #594054 -
Flags: review?(jones.chris.g)
Flags: in-testsuite+
Assignee | ||
Comment 1•9 years ago
|
||
I thought Chris had helped work on CTypes, and thus r?'d him; if that's not correct, apologies; who is best?
Updated•9 years ago
|
Attachment #594054 -
Flags: review?(jones.chris.g) → review?(bobbyholley+bmo)
Comment 2•9 years ago
|
||
Comment on attachment 594054 [details] [diff] [review] Make JavaScript CTypes print function types correctly. >- // Wrap the entire expression so far with parens. >- PrependString(result, "("); >- AppendString(result, ")"); >+ // Function application binds more tightly than dereferencing, so >+ // wrap pointer types in parens. >+ if (prevGrouping == TYPE_pointer) { >+ PrependString(result, "("); >+ AppendString(result, ")"); >+ } Good catch. :-) Could be worth making a note here that pointers are the only modifiers that can directly wrap functions (since you can't have arrays of functions or functions returning functions), so the only thing we're guarding against here is function types as the outermost modifier. But it's not super crucial. >diff --git a/js/src/jit-test/tests/basic/CTypes-toString.js b/js/src/jit-test/tests/basic/CTypes-toString.js Awesome tests! These should go in toolkit/components/ctypes/tests though. r=bholley with the tests moved.
Attachment #594054 -
Flags: review?(bobbyholley+bmo) → review+
Assignee | ||
Comment 3•9 years ago
|
||
Revised per bholley's review comments.
Attachment #594054 -
Attachment is obsolete: true
Assignee | ||
Comment 4•9 years ago
|
||
try run: https://tbpl.mozilla.org/?tree=Try&rev=4eee592f6378
Assignee | ||
Comment 5•9 years ago
|
||
Try looks good. https://hg.mozilla.org/integration/mozilla-inbound/rev/c0077ffef801
Comment 6•9 years ago
|
||
Backed out due to test failures on all Windows boxes in test_ctypes.js and test_ctypes.xul. Fwiw, Try didn't really look good ;)
Assignee | ||
Comment 7•9 years ago
|
||
(In reply to Marco Bonardo [:mak] from comment #6) > Fwiw, Try didn't really look good ;) I suck. Thanks, Marco.
Assignee | ||
Comment 8•9 years ago
|
||
Revised to fix Win32 tests (bad spacing for __stdcall and WINAPI qualifiers).
Attachment #594854 -
Attachment is obsolete: true
Assignee | ||
Comment 9•9 years ago
|
||
New try run: https://tbpl.mozilla.org/?tree=Try&rev=c327706ed342
Assignee | ||
Comment 10•9 years ago
|
||
(In reply to Jim Blandy :jimb from comment #9) > New try run: https://tbpl.mozilla.org/?tree=Try&rev=c327706ed342 That showed up some more xpcshell test failures. This one is clean: https://tbpl.mozilla.org/?tree=Try&rev=cc27c2c3fa1a
Assignee | ||
Comment 11•9 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/f80c406b902a
Comment 12•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/f80c406b902a
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•