763860 - xpidl needs to reject overloaded methods in scriptable interfaces even if the first letter is different case

Status: NEW

(Core :: XPCOM, defect)

Description

[Aryeh Gregor (:ayg) (no longer with Mozilla)]

See bug 762841.  Attachment 631846 [details] [diff] there did

     nsISelection getSelection(in short type);
+    [noscript,notxpcom,nostdcall] Selection GetSelection(in short type);

When I wrote "getSelection" for the second method, it complained that there was a naming conflict, but when I capitalized it, it worked happily.  But then it broke horribly on Windows, because the generated C++ methods had the same name and that breaks on Windows for scriptable interfaces for some reason.  The parser should refuse to do this.

Comment 1

[Boris Zbarsky [:bzbarsky]]

It breaks on windows for any interface if you call it through XPConnect, because MSVC will reorder the members in some cases and then the vtable info XPConnect no longer matches the actual vtable.

Comment 2

[Mats Palmgren (inactive)]

Boris, does that apply to overloaded methods only?
or can that error occur for any [noscript,notxpcom,nostdcall]
method in a scriptable interface?

Comment 3

[Boris Zbarsky [:bzbarsky]]

That applies to overloaded methods.

Comment 4

[(no longer active)]

Note that if all of the overloaded methods are noscript/notxpcom (whichever makes them impossible to get called through xpconnect) this should be fine...  Although I personally wouldn't go through the trouble of special casing that!

Comment 5

[Mats Palmgren (inactive)]

(In reply to Boris Zbarsky (:bz) from comment #1)
> It breaks on windows for any interface if you call it through XPConnect,
> because MSVC will reorder the members in some cases and then the vtable info
> XPConnect no longer matches the actual vtable.

Is that the case also if we use two separate classes?

If we generate the scriptable class with a different name:

class NS_NO_VTABLE NS_SCRIPTABLE nsISelectionControllerScriptable 
  : public nsISelectionDisplay {
   ... IDL stuff ...
};

and then declare the internal type like so:

class NS_NO_VTABLE nsISelectionController
  : public nsISelectionControllerScriptable {
 public: 
  using nsISelectionControllerS::GetSelection;
  virtual already_AddRefed<mozilla::Selection> GetSelection(PRInt16 type) = 0;
};


I tested this briefly on Linux and Win7 and it seemed to work...

Comment 6

[Boris Zbarsky [:bzbarsky]]

> Is that the case also if we use two separate classes?

No, that can't be reordered, because the superclass vtable needs to not include the subclass methods.

Comment 7

[Mats Palmgren (inactive)]

Attachment: part 1, implement 'separate_scriptable_name' interface attribute

This patch implements a 'separate_scriptable_name' interface attribute
that generates a separate class with the name given name and only
includes scriptable methods/attributes and constants in that class.
The non-scriptable stuff goes into a sub-class.
I've also added a 'overloaded' method attribute to indicate an
scriptable/non-scriptable overloaded method (to generate the
"using ..." bit), but this is probably redundant since it can be
deduced from the internal method table I think.

Comment 8

[Mats Palmgren (inactive)]

Attachment: part 2, an example use for nsISelectionController::GetSelection (bug 762841)

Comment 9

[Mats Palmgren (inactive)]

Attachment: the generated nsISelectionController.h with the above patches