Re: *.idl: Use correct type for BSTR strings in the defaultvalue() statements.
On 06.11.2015 11:37, Dmitry Timoshkov wrote:
This matches midl/PSDK syntax, and allows to use Wine headers for compiling applications with a C++ win32/win64 compiler. In order to be usable by C++ unix compilers widl needs to emit appropriate statically initialized WCHAR strings, but that's a different problem, currently the generated Wine headers with default(BSTR) statements can't be used with C++ at all.
Where do you see this? Mshtml.idl from Windows 10 SDK doesn't use this notation, and in fact only fsrm*.idl files do, so it's not a common practice. Generated headers should use L"", like you said. But only in case of empty strings. Also leaving this broken for everything that's not PSDK compiler doesn't sound right.
Nikolay Sivov <bunglehead(a)gmail.com> wrote:
This matches midl/PSDK syntax, and allows to use Wine headers for compiling applications with a C++ win32/win64 compiler. In order to be usable by C++ unix compilers widl needs to emit appropriate statically initialized WCHAR strings, but that's a different problem, currently the generated Wine headers with default(BSTR) statements can't be used with C++ at all.
Where do you see this? Mshtml.idl from Windows 10 SDK doesn't use this notation, and in fact only fsrm*.idl files do, so it's not a common practice.
midl has a built-in "magic" for handling defaultvalue() for BSTR strings, that's why PSDK headers are not consistent in that regard: some of them use defaultvalue("") while others use defaultvalue(L""). But using this magic is not necessary if the header uses appropriate type for the default value. So fixing an .idl should be preferrable IMO to adding support for magic behaviour to widl.
Generated headers should use L"", like you said. But only in case of empty strings.
midl always generates L"" syntax, regardless whether it's an empty string or not.
Also leaving this broken for everything that's not PSDK compiler doesn't sound right.
mingw/borland/any other wine32 compiler also benefit from this change. It doesn't make sense to have it broken at all, but at least fixing the .idl side of breakage should be considered as the first step, and make the generated headers actually usable for the win32 platform. -- Dmitry.
On Friday, 6 November 2015, Dmitry Timoshkov <dmitry(a)baikal.ru> wrote:
Nikolay Sivov <bunglehead(a)gmail.com <javascript:;>> wrote:
This matches midl/PSDK syntax, and allows to use Wine headers for compiling applications with a C++ win32/win64 compiler. In order to be usable by C++ unix compilers widl needs to emit appropriate statically initialized WCHAR strings, but that's a different problem, currently the generated Wine headers with default(BSTR) statements can't be used with C++ at all.
Where do you see this? Mshtml.idl from Windows 10 SDK doesn't use this notation, and in fact only fsrm*.idl files do, so it's not a common practice.
midl has a built-in "magic" for handling defaultvalue() for BSTR strings, that's why PSDK headers are not consistent in that regard: some of them use defaultvalue("") while others use defaultvalue(L""). But using this magic is not necessary if the header uses appropriate type for the default value. So fixing an .idl should be preferrable IMO to adding support for magic behaviour to widl.
All headers except about 4 we don't have, use it like we do. All the magic is to add a prefix during header generation, it doesn't look that hard, and will get consistent result for idls that don't cone with wine.
Generated headers should use L"", like you said. But only in case of empty strings.
midl always generates L"" syntax, regardless whether it's an empty string or not.
I don't see this in mshtml.h when default value is not an empty string.
Also leaving this broken for everything that's not PSDK compiler doesn't sound right.
mingw/borland/any other wine32 compiler also benefit from this change. It doesn't make sense to have it broken at all, but at least fixing the .idl side of breakage should be considered as the first step, and make the generated headers actually usable for the win32 platform.
Since it only affects cpp case, that we don't use, how does it help
building wine? And more importantly what will happen to gcc if you try to build with this addition?
-- Dmitry.
Nikolay Sivov <bunglehead(a)gmail.com> wrote:
Nikolay Sivov <bunglehead(a)gmail.com <javascript:;>> wrote:
This matches midl/PSDK syntax, and allows to use Wine headers for compiling applications with a C++ win32/win64 compiler. In order to be usable by C++ unix compilers widl needs to emit appropriate statically initialized WCHAR strings, but that's a different problem, currently the generated Wine headers with default(BSTR) statements can't be used with C++ at all.
Where do you see this? Mshtml.idl from Windows 10 SDK doesn't use this notation, and in fact only fsrm*.idl files do, so it's not a common practice.
midl has a built-in "magic" for handling defaultvalue() for BSTR strings, that's why PSDK headers are not consistent in that regard: some of them use defaultvalue("") while others use defaultvalue(L""). But using this magic is not necessary if the header uses appropriate type for the default value. So fixing an .idl should be preferrable IMO to adding support for magic behaviour to widl.
All headers except about 4 we don't have, use it like we do. All the magic is to add a prefix during header generation, it doesn't look that hard, and will get consistent result for idls that don't cone with wine.
I don't really follow your objections. It's pretty clear that Wine .idl files with defaultvalue("") statements for BSTR type are broken. There are two ways to fix them: 1) fix .idl files, widl and midl already have proper support for L"" syntax. 2) don't fix .idl files (just because PSDK has them that way) and add magic handling for BSTR to widl. Fixing .idl files is a natural and easy fix, what syntax uses PSDK doesn't matter al all, it's even better to differ from PSDK headers in this case.
Generated headers should use L"", like you said. But only in case of empty strings.
midl always generates L"" syntax, regardless whether it's an empty string or not.
I don't see this in mshtml.h when default value is not an empty string.
Probably midl doesn't correctly handle even the magic that it's supposed to support. That just confirms that adding "magic" support for defaultvalue() to widl wouldn't solve the problem completely, and support for this "magic" would need adding even more magic to match midl brokeness.
Also leaving this broken for everything that's not PSDK compiler doesn't sound right.
mingw/borland/any other wine32 compiler also benefit from this change. It doesn't make sense to have it broken at all, but at least fixing the .idl side of breakage should be considered as the first step, and make the generated headers actually usable for the win32 platform.
Since it only affects cpp case, that we don't use, how does it help building wine?
And more importantly what will happen to gcc if you try to build with this addition?
gcc does build generated headers just fine. I wonder why you don't care what happens to gcc when it builds with current broken headers. -- Dmitry.
On Friday, 6 November 2015, Dmitry Timoshkov <dmitry(a)baikal.ru> wrote:
Nikolay Sivov <bunglehead(a)gmail.com <javascript:;>> wrote:
Nikolay Sivov <bunglehead(a)gmail.com <javascript:;> <javascript:;>> wrote:
This matches midl/PSDK syntax, and allows to use Wine headers for compiling applications with a C++ win32/win64 compiler. In order to be usable by C++ unix compilers widl needs to emit appropriate statically initialized WCHAR strings, but that's a different problem, currently the generated Wine headers with default(BSTR) statements can't be used with C++ at all.
Where do you see this? Mshtml.idl from Windows 10 SDK doesn't use this notation, and in fact only fsrm*.idl files do, so it's not a common practice.
midl has a built-in "magic" for handling defaultvalue() for BSTR strings, that's why PSDK headers are not consistent in that regard: some of them use defaultvalue("") while others use defaultvalue(L""). But using this magic is not necessary if the header uses appropriate type for the default value. So fixing an .idl should be preferrable IMO to adding support for magic behaviour to widl.
All headers except about 4 we don't have, use it like we do. All the magic is to add a prefix during header generation, it doesn't look that hard, and will get consistent result for idls that don't cone with wine.
I don't really follow your objections. It's pretty clear that Wine .idl files with defaultvalue("") statements for BSTR type are broken. There are two ways to fix them: 1) fix .idl files, widl and midl already have proper support for L"" syntax. 2) don't fix .idl files (just because PSDK has them that way) and add magic handling for BSTR to widl. Fixing .idl files is a natural and easy fix, what syntax uses PSDK doesn't matter al all, it's even better to differ from PSDK headers in this case.
Assigning static string in attempt to initialize BSTR is broken too of course.
Generated headers should use L"", like you said. But only in case of empty strings.
midl always generates L"" syntax, regardless whether it's an empty string or not.
I don't see this in mshtml.h when default value is not an empty string.
Probably midl doesn't correctly handle even the magic that it's supposed to support. That just confirms that adding "magic" support for defaultvalue() to widl wouldn't solve the problem completely, and support for this "magic" would need adding even more magic to match midl brokeness.
Also leaving this broken for everything that's not PSDK compiler doesn't sound right.
mingw/borland/any other wine32 compiler also benefit from this change. It doesn't make sense to have it broken at all, but at least fixing the .idl side of breakage should be considered as the first step, and make the generated headers actually usable for the win32 platform.
Since it only affects cpp case, that we don't use, how does it help building wine?
And more importantly what will happen to gcc if you try to build with this addition?
gcc does build generated headers just fine. I wonder why you don't care what happens to gcc when it builds with current broken headers.
Currently our headers don't have L"" strings, and we don't use it anywhere.
-- Dmitry.
Nikolay Sivov <bunglehead(a)gmail.com> wrote:
This matches midl/PSDK syntax, and allows to use Wine headers for compiling applications with a C++ win32/win64 compiler. In order to be usable by C++ unix compilers widl needs to emit appropriate statically initialized WCHAR strings, but that's a different problem, currently the generated Wine headers with default(BSTR) statements can't be used with C++ at all.
Where do you see this? Mshtml.idl from Windows 10 SDK doesn't use this notation, and in fact only fsrm*.idl files do, so it's not a common practice.
midl has a built-in "magic" for handling defaultvalue() for BSTR strings, that's why PSDK headers are not consistent in that regard: some of them use defaultvalue("") while others use defaultvalue(L""). But using this magic is not necessary if the header uses appropriate type for the default value. So fixing an .idl should be preferrable IMO to adding support for magic behaviour to widl.
All headers except about 4 we don't have, use it like we do. All the magic is to add a prefix during header generation, it doesn't look that hard, and will get consistent result for idls that don't cone with wine.
I don't really follow your objections. It's pretty clear that Wine .idl files with defaultvalue("") statements for BSTR type are broken. There are two ways to fix them: 1) fix .idl files, widl and midl already have proper support for L"" syntax. 2) don't fix .idl files (just because PSDK has them that way) and add magic handling for BSTR to widl. Fixing .idl files is a natural and easy fix, what syntax uses PSDK doesn't matter al all, it's even better to differ from PSDK headers in this case.
Assigning static string in attempt to initialize BSTR is broken too of course.
It works just fine. And since midl generates exactly this (L"blah") for defaultvalue() strings that's how it's supposed to work.
Generated headers should use L"", like you said. But only in case of empty strings.
midl always generates L"" syntax, regardless whether it's an empty string or not.
I don't see this in mshtml.h when default value is not an empty string.
Probably midl doesn't correctly handle even the magic that it's supposed to support. That just confirms that adding "magic" support for defaultvalue() to widl wouldn't solve the problem completely, and support for this "magic" would need adding even more magic to match midl brokeness.
Also leaving this broken for everything that's not PSDK compiler doesn't sound right.
mingw/borland/any other wine32 compiler also benefit from this change. It doesn't make sense to have it broken at all, but at least fixing the .idl side of breakage should be considered as the first step, and make the generated headers actually usable for the win32 platform.
Since it only affects cpp case, that we don't use, how does it help building wine?
And more importantly what will happen to gcc if you try to build with this addition?
gcc does build generated headers just fine. I wonder why you don't care what happens to gcc when it builds with current broken headers.
Currently our headers don't have L"" strings, and we don't use it anywhere.
Having L"" for BSTR strings is a lot better than current situation. It provides a working solution for win32 compilers, and gives some choice to native compilers (-fshort-wchar and similar). Currently nothing compiles at all under either platform. -- Dmitry.
On 11/06/15 13:10, Dmitry Timoshkov wrote:
I don't really follow your objections. It's pretty clear that Wine .idl files with defaultvalue("") statements for BSTR type are broken. There are two ways to fix them: 1) fix .idl files, widl and midl already have proper support for L"" syntax. 2) don't fix .idl files (just because PSDK has them that way) and add magic handling for BSTR to widl. Fixing .idl files is a natural and easy fix, what syntax uses PSDK doesn't matter al all, it's even better to differ from PSDK headers in this case.
Well, out of those two options I'd prefer option 1. That's a real bug. Option 2 just changes IDLs that are perfectly fine, just misinterpreted by widl. FWIW think that it's all broken really badly by Microsoft. BSTR strings are supposed to be allocated by SysAllocString* allocators and can't be static strings. It will fail as soon as someone tries things like SysStringLen() call on it. What midl does is promoting broken use of BSTR type. But, well, we should follow that for compatibility :/
> Generated headers should use L"", like you said. But only in > case of empty strings.
midl always generates L"" syntax, regardless whether it's an empty string or not.
I don't see this in mshtml.h when default value is not an empty string. Probably midl doesn't correctly handle even the magic that it's supposed to support. That just confirms that adding "magic" support for defaultvalue() to widl wouldn't solve the problem completely, and support for this "magic" would need adding even more magic to match midl brokeness.
You both seem to misinterpret what's heppening with mshtm.idl. C++ default values can be emitted only for last arguments. There may be a few of them, but they can't be followed by an argument without a default value. That's why midl doesn't emit default values for non-empty strings found in mshtml.idl. I checked that with tests and midl emits L"..." strings as expected. Cheers, Jacek
Jacek Caban <jacek(a)codeweavers.com> wrote:
I don't really follow your objections. It's pretty clear that Wine .idl files with defaultvalue("") statements for BSTR type are broken. There are two ways to fix them: 1) fix .idl files, widl and midl already have proper support for L"" syntax. 2) don't fix .idl files (just because PSDK has them that way) and add magic handling for BSTR to widl. Fixing .idl files is a natural and easy fix, what syntax uses PSDK doesn't matter al all, it's even better to differ from PSDK headers in this case.
Well, out of those two options I'd prefer option 1. That's a real bug. Option 2 just changes IDLs that are perfectly fine, just misinterpreted by widl.
FWIW think that it's all broken really badly by Microsoft. BSTR strings are supposed to be allocated by SysAllocString* allocators and can't be static strings. It will fail as soon as someone tries things like SysStringLen() call on it. What midl does is promoting broken use of BSTR type.
Yes, BSTRs are length-prefixed WCHAR strings, but BSTR and WCHAR* are often freely interchanged in the APIs, and that's not our fault.
But, well, we should follow that for compatibility :/
Not the first time :)
> > Generated headers should use L"", like you said. But only in > > case of empty strings.
midl always generates L"" syntax, regardless whether it's an empty string or not.
I don't see this in mshtml.h when default value is not an empty string. Probably midl doesn't correctly handle even the magic that it's supposed to support. That just confirms that adding "magic" support for defaultvalue() to widl wouldn't solve the problem completely, and support for this "magic" would need adding even more magic to match midl brokeness.
You both seem to misinterpret what's heppening with mshtm.idl. C++ default values can be emitted only for last arguments. There may be a few of them, but they can't be followed by an argument without a default value. That's why midl doesn't emit default values for non-empty strings found in mshtml.idl. I checked that with tests and midl emits L"..." strings as expected.
Thanks for explaining Jacek, much appreciated. -- Dmitry.
On 11/06/15 14:21, Jacek Caban wrote:
On 11/06/15 13:10, Dmitry Timoshkov wrote:
I don't really follow your objections. It's pretty clear that Wine .idl files with defaultvalue("") statements for BSTR type are broken. There are two ways to fix them: 1) fix .idl files, widl and midl already have proper support for L"" syntax. 2) don't fix .idl files (just because PSDK has them that way) and add magic handling for BSTR to widl. Fixing .idl files is a natural and easy fix, what syntax uses PSDK doesn't matter al all, it's even better to differ from PSDK headers in this case. Well, out of those two options I'd prefer option 1. That's a real bug. Option 2 just changes IDLs that are perfectly fine, just misinterpreted by widl.
I obviously messed option number. I meant I prefer option 2, as could be guessed by explanation. Thanks Huw for pointing it out. Cheers, Jacek
participants (3)
-
Dmitry Timoshkov -
Jacek Caban -
Nikolay Sivov