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
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
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
participants (2)
-
Jacek Caban -
Thomas Faber