Re: widl: Do not generate C structure for empty interfaces. (try 2)
On 06/30/13 23:14, Thomas Faber wrote:
On 2013-06-27 13:33, Jacek Caban wrote:
That
said, if such interfaces are just obscure special case, I would say we
shouldn't care. We may easily avoid them (I just sent a patch avoiding
it in mshtml).
Ah, I hadn't even considered changing the interface. Seemed like it was
something mandated by Mozilla's use of it.
This indeed seems to be the best solution to the immediate problem of
mshtml not building with MSVC.
Thanks!
In general, Mozilla uses their own variant of COM, called XPCOM. It
happens to be similar enough to MS variant of COM that we may use widl
to generate its binary compatible headers, but if there is any
incompatibility, fixing it in nsiface.idl is preferred over hacking widl.
If you want a better solution to avoid such problems in the future, I
would suggest adding an error, like midl does. This will, however,
require some more changes in Wine. At least MSHTML already has an
interface identical to IUnknown (nsISupports), which would cause the
error. This should be solvable with some tricks.
That sounds like the smart thing to do. I've attached the simplest
version that comes to mind.
The interfaces I needed to exempt were IUnknown (naturally),
ID3DInclude (which PSDK seems to define via cpp_quote), and nsISupports
as you already mentioned.
Midl also seems to have an if not inside a library condition, which
I'll try to add as well. I'd have liked to use is_object in widl, but
that also triggers on the odl attribute.
I'll send it to -patches once I'm satisfied with it, but I'm pretty
fond of the simple approach already. Let me know if you have any
comments.
I don't think we want to maintain such a list of special interfaces in
widl sources. IUnknown is a special case in many aspects, so that's
fine. ID3DInclude is something that definitely shouldn't be there. It's
also small enough that cpp_quote should be fine for this. nsISupports
is, as I said earlier, something that would be better worked around in
nsiface.idl. The attached patch does that, although now there will
probably be duplicated UUID error on midl. That's also something we
could work around with some preprocessor tricks...
If this would be generic enough problem, we could add some widl
extension, like a special attribute, but your findings show that it's
too rare to mitigate that.
Thanks,
Jacek
diff --git a/dlls/mshtml/nsiface.idl b/dlls/mshtml/nsiface.idl
index fcdf6f1..3bbcd46 100644
--- a/dlls/mshtml/nsiface.idl
+++ b/dlls/mshtml/nsiface.idl
@@ -26,6 +26,7 @@
cpp_quote(#define GECKO_VERSION \2.21\)
cpp_quote(#define GECKO_VERSION_STRING \Wine Gecko \ GECKO_VERSION)
+import unknwn.idl;
import wtypes.idl;
cpp_quote(#ifdef WINE_NO_UNICODE_MACROS)
@@ -114,12 +115,8 @@ interface IMoniker;
uuid(---c000-0046),
local
]
-interface nsISupports
-{
-nsresult QueryInterface(nsIIDRef riid, void **result);
-nsrefcnt AddRef();
-nsrefcnt Release();
-}
+interface nsISupports : IUnknown
+{}
/* Currently we don't need a full declaration of these interfaces */
typedef nsISupports nsISHistory;
Re: widl: Do not generate C structure for empty interfaces. (try 2)
On 2013-06-27 13:33, Jacek Caban wrote:
That
said, if such interfaces are just obscure special case, I would say we
shouldn't care. We may easily avoid them (I just sent a patch avoiding
it in mshtml).
Ah, I hadn't even considered changing the interface. Seemed like it was
something mandated by Mozilla's use of it.
This indeed seems to be the best solution to the immediate problem of
mshtml not building with MSVC.
Thanks!
If you want a better solution to avoid such problems in the future, I
would suggest adding an error, like midl does. This will, however,
require some more changes in Wine. At least MSHTML already has an
interface identical to IUnknown (nsISupports), which would cause the
error. This should be solvable with some tricks.
That sounds like the smart thing to do. I've attached the simplest
version that comes to mind.
The interfaces I needed to exempt were IUnknown (naturally),
ID3DInclude (which PSDK seems to define via cpp_quote), and nsISupports
as you already mentioned.
Midl also seems to have an if not inside a library condition, which
I'll try to add as well. I'd have liked to use is_object in widl, but
that also triggers on the odl attribute.
I'll send it to -patches once I'm satisfied with it, but I'm pretty
fond of the simple approach already. Let me know if you have any
comments.
Thanks a lot.
-Thomas
From 55a00aae2900cec493b06d6a434809f053d76fe5 Mon Sep 17 00:00:00 2001
From: Thomas Faber thfa...@gmx.de
Date: Sun, 30 Jun 2013 19:42:32 +0200
Subject: widl: Require interfaces to inherit from another interface unless
explicitly exempted.
---
tools/widl/typetree.c | 12
1 file changed, 12 insertions(+)
diff --git a/tools/widl/typetree.c b/tools/widl/typetree.c
index a9e71be..de373a2 100644
--- a/tools/widl/typetree.c
+++ b/tools/widl/typetree.c
@@ -352,6 +352,14 @@ type_t *type_new_bitfield(type_t *field, const expr_t
*bits)
return t;
}
+static int is_root_interface(type_t *iface)
+{
+if (!strcmp(iface-name, IUnknown)) return TRUE;
+if (!strcmp(iface-name, ID3DInclude)) return TRUE;
+if (!strcmp(iface-name, nsISupports)) return TRUE;
+return FALSE;
+}
+
static int compute_method_indexes(type_t *iface)
{
int idx;
@@ -377,11 +385,15 @@ static int compute_method_indexes(type_t *iface)
void type_interface_define(type_t *iface, type_t *inherit, statement_list_t
*stmts)
{
+const attr_t *attr;
iface-details.iface = xmalloc(sizeof(*iface-details.iface));
iface-details.iface-disp_props = NULL;
iface-details.iface-disp_methods = NULL;
iface-details.iface-stmts = stmts;
iface-details.iface-inherit = inherit;
+if (iface-attrs) LIST_FOR_EACH_ENTRY( attr, iface-attrs, const attr_t,
entry )
+if (attr-type == ATTR_OBJECT !inherit !is_root_interface(iface))
+error_loc(object interface must inherit from another object
interface\n);
iface-defined = TRUE;
compute_method_indexes(iface);
}
--
1.8.1.5
Re: widl: Do not generate C structure for empty interfaces. (try 2)
Hi Thomas, I tested what midl does in this case and I think the solution should be different. When I tried to compile an empty interface, I got an error saying that '[object] interface must derive from another [object] interface'. That requirement is mostly enough to avoid empty vtbl. I was wondering how IUnknown is handled by midl (since it doesn't derive from any other iface). It seems like it's a simple name-based special case. When I renamed the interface to IUnknown, it compiled fine and generated vtbl the same as current widl (that is an empty struct). That said, if such interfaces are just obscure special case, I would say we shouldn't care. We may easily avoid them (I just sent a patch avoiding it in mshtml). If you want a better solution to avoid such problems in the future, I would suggest adding an error, like midl does. This will, however, require some more changes in Wine. At least MSHTML already has an interface identical to IUnknown (nsISupports), which would cause the error. This should be solvable with some tricks. Thanks, Jacek
