Closed Bug 648871 Opened 13 years ago Closed 13 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) — 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) — 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) — 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
http://hg.mozilla.org/projects/cedar/rev/1b88843c7591
Keywords: checkin-needed
Whiteboard: fixed-in-cedar
Target Milestone: --- → mozilla6
http://hg.mozilla.org/mozilla-central/rev/1b88843c7591
Status: ASSIGNED → RESOLVED
Closed: 13 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: