Closed Bug 648871 Opened 14 years ago Closed 14 years ago

Mismatched calling convention for CanvasStyleSetterType

Categories

(Core :: Graphics: Canvas2D, defect)

x86
macOS
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla6

People

(Reporter: espindola, Assigned: espindola)

References

Details

Attachments

(1 file, 3 obsolete files)

Attached patch Change the calling convention (obsolete) (deleted) — Splinter Review
CanvasStyleSetterType and CanvasStyleSetterType are declared with NS_STDCALL but are used with methods declared with NS_DEFCALL. Clang complains with: In file included from dom_quickstubs.cpp:241:../../../../dist/include/CustomQS_Canvas2D.h:139:12: error: no matching function for call to 'Canvas2D_SetStyleHelper' return Canvas2D_SetStyleHelper(cx, obj, id, vp, &nsIDOMCanvasRenderingContext2D::SetStrokeStyle_multi); ^~~~~~~~~~~~~~~~~~~~~~~ ../../../../dist/include/CustomQS_Canvas2D.h:51:1: note: candidate function not viable: no known conversion from 'nsresult (nsIDOMCanvasRenderingContext2D::*)(const nsAString_internal &, nsISupports *) __attr ibute__((cdecl))' to 'CanvasStyleSetterType' (aka 'nsresult (nsIDOMCanvasRenderingContext2D::*)(const nsAString_internal &, nsISupports *)') for 5th argument Canvas2D_SetStyleHelper(JSContext *cx, JSObject *obj, jsid id, jsval *vp, ^
Attachment #524963 - Flags: review?
Attached patch Change the calling convention (obsolete) (deleted) — Splinter Review
Attachment #524963 - Attachment is obsolete: true
Attachment #524963 - Flags: review?
Attachment #524965 - Flags: review?
Uh... nsCanvasRenderingContext2D::SetStrokeStyle_multi is stdcall, no? At least in all situations in which the typedef is stdcall. Or put another way, does your patch compile on Windows?
Oh, I see what's going on here. SetStrokeStyle_multi is defined as NS_IMETHOD, which means one of the following things: __stdcall (on Windows) nothing special (on OS/2) NS_DEFCALL (elsewhere) When __GNUC__ is defined on i386, NS_DEFCALL expands to: __attribute__ ((regparm (0), cdecl)) and otherwise it means nothing special. The typedef is using NS_STDCALL, which means the following: __stdcall on windows nothing special elsewhere So the current code is correct on Windows (and your new code is wrong there), but broken on a compiler that defines __GNUC__. The right thing to do here seems to be to use the NS_STDCALL_FUNCPROTO macros to define the typedefs, right? At least assuming the __GNUC__ bit of those works in clang.
Also, you should be requesting review from specific people if you want the review request to be seen. :(
Summary: Mismatched calling convention → Mismatched calling convention for CanvasStyleSetterType
Blocks: 648872
Attached patch NS_STDCALL_FUNCPROTO (obsolete) (deleted) — Splinter Review
Attachment #524965 - Attachment is obsolete: true
Attachment #524965 - Flags: review?
Attachment #524970 - Flags: review?
Attachment #524970 - Flags: review? → review?(bzbarsky)
Comment on attachment 524970 [details] [diff] [review] NS_STDCALL_FUNCPROTO r=me. Sorry for the lag here...
Attachment #524970 - Flags: review?(bzbarsky) → review+
Assignee: nobody → respindola
Rafael, could you please prepare patches as described on <https://developer.mozilla.org/en/Mercurial_FAQ#How_can_I_generate_a_patch_for_somebody_else_to_check-in_for_me.3f>? That would make it much easier to push them.
Status: NEW → ASSIGNED
Attached patch hg generated patch (deleted) — Splinter Review
Attachment #524970 - Attachment is obsolete: true
Keywords: checkin-needed
Whiteboard: fixed-in-cedar
Target Milestone: --- → mozilla6
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Whiteboard: fixed-in-cedar
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: