Hey everyone,
during the course of implementing WinRT / UWP stuff, I regularly come across missing IDL 3.0 features in our Widl compiler. Unfortunately, Widl's code seems to have grown in a rater unpleasant direction and is sometimes a real burden to understand and maintain. I think some refactoring to its code would be very well fitting and make things a lot easier for everyone, but refactoring can also bring breakage and makes tracking down errors with Git harder. Additionally, as the code is already not that small, errors have an easy way to slip in without noticing. So to me, the ideal solution seems to add a testing system to the compiler, like most of Wine's components already have.
Rémi and I already outlined some ideas: - Port the Winetest system to Unix programs - Make a libwidl or Widl.exe and test against that with our current test system - Add the tests into Widl itself as callable parameter
What do you think is the best solution? Getting some suggestions would be great. :)
Thanks, Bernhard
On Wed, Jun 15, 2022 at 05:20:00PM +0200, Bernhard Kölbl wrote:
Hey everyone,
during the course of implementing WinRT / UWP stuff, I regularly come across missing IDL 3.0 features in our Widl compiler. Unfortunately, Widl's code seems to have grown in a rater unpleasant direction and is sometimes a real burden to understand and maintain. I think some refactoring to its code would be very well fitting and make things a lot easier for everyone, but refactoring can also bring breakage and makes tracking down errors with Git harder. Additionally, as the code is already not that small, errors have an easy way to slip in without noticing. So to me, the ideal solution seems to add a testing system to the compiler, like most of Wine's components already have.
Rémi and I already outlined some ideas:
- Port the Winetest system to Unix programs
- Make a libwidl or Widl.exe and test against that with our current test system
- Add the tests into Widl itself as callable parameter
What do you think is the best solution? Getting some suggestions would be great. :)
Thanks, Bernhard
I don't have any suggestions or anything, just wanted to say I think this is a good idea as well and something I've thought about in the past. There have been a few times where I've wanted to touch widl and been a bit scared off of it due to fear of breaking a bunch of things by accident. Any sort of test system would be great to prevent these kinds of issues. It'd also help when adding support for new features.
Sorry for not having any suggestions, just wanted to voice my support for something like this. :)
On 6/15/22 18:20, Bernhard Kölbl wrote:
Hey everyone,
during the course of implementing WinRT / UWP stuff, I regularly come across missing IDL 3.0 features in our Widl compiler. Unfortunately, Widl's code seems to have grown in a rater unpleasant direction and is sometimes a real burden to understand and maintain. I think some refactoring to its code would be very well fitting and make things a lot easier for everyone, but refactoring can also bring breakage and makes tracking down errors with Git harder. Additionally, as the code is already not that small, errors have an easy way to slip in without noticing. So to me, the ideal solution seems to add a testing system to the compiler, like most of Wine's components already have.
Rémi and I already outlined some ideas:
- Port the Winetest system to Unix programs
- Make a libwidl or Widl.exe and test against that with our current test system
- Add the tests into Widl itself as callable parameter
What do you think is the best solution? Getting some suggestions would be great. :)
There are options. First it depends whether you want to validate against midl or not. If you do then you'll need something like widl.exe to run on Windows.
Next question is what your test data would be, and how to compare the output reliably with expected output. For example we don't really need exact matching for generated headers, or even marshaling format strings, as long as they achieve their purpose. For typelibs however, it might be useful to go for exact binary match of selected sections.
If testing against midl is not a goal, we could have tests for widl itself, even in form of scripts, to run on host system, and not through wine. This way you'll need expected test data that you trust, and maintain in a good shape without validating it on Windows. Even if it doesn't match midl output exactly, it's still potentially useful to catch regressions.
Thanks, Bernhard
On 6/15/22 12:49, Nikolay Sivov wrote:
On 6/15/22 18:20, Bernhard Kölbl wrote:
Hey everyone,
during the course of implementing WinRT / UWP stuff, I regularly come across missing IDL 3.0 features in our Widl compiler. Unfortunately, Widl's code seems to have grown in a rater unpleasant direction and is sometimes a real burden to understand and maintain. I think some refactoring to its code would be very well fitting and make things a lot easier for everyone, but refactoring can also bring breakage and makes tracking down errors with Git harder. Additionally, as the code is already not that small, errors have an easy way to slip in without noticing. So to me, the ideal solution seems to add a testing system to the compiler, like most of Wine's components already have.
Rémi and I already outlined some ideas:
- Port the Winetest system to Unix programs
- Make a libwidl or Widl.exe and test against that with our current
test system
- Add the tests into Widl itself as callable parameter
What do you think is the best solution? Getting some suggestions would be great. :)
There are options. First it depends whether you want to validate against midl or not. If you do then you'll need something like widl.exe to run on Windows.
Next question is what your test data would be, and how to compare the output reliably with expected output. For example we don't really need exact matching for generated headers, or even marshaling format strings, as long as they achieve their purpose. For typelibs however, it might be useful to go for exact binary match of selected sections.
We already have tests for typelibs and marshalling, in oleaut32:typelib and oleaut32:tmarshal respectively. I see no reason that's not sufficient already.
As far as headers are concerned, we could probably take a similar approach in some ways. I don't know what exactly needs testing, but we could do things like
ok(offsetof(mystruct.field) == 16, "wrong offset %u\n", offsetof(mystruct.field));
or (for a compile-time assertion)
extern void test(ITestInterface *obj) { HRESULT (*myfunc)(type1 arg1, type2 *arg2, ...);
myfunc = obj->lpVtbl->myfunc; }
Ultimately we may not need to mess with widl, or write any extra test infrastructure.
On 6/15/22 20:04, Zebediah Figura wrote:
On 6/15/22 12:49, Nikolay Sivov wrote:
On 6/15/22 18:20, Bernhard Kölbl wrote:
Hey everyone,
during the course of implementing WinRT / UWP stuff, I regularly come across missing IDL 3.0 features in our Widl compiler. Unfortunately, Widl's code seems to have grown in a rater unpleasant direction and is sometimes a real burden to understand and maintain. I think some refactoring to its code would be very well fitting and make things a lot easier for everyone, but refactoring can also bring breakage and makes tracking down errors with Git harder. Additionally, as the code is already not that small, errors have an easy way to slip in without noticing. So to me, the ideal solution seems to add a testing system to the compiler, like most of Wine's components already have.
Rémi and I already outlined some ideas:
- Port the Winetest system to Unix programs
- Make a libwidl or Widl.exe and test against that with our current
test system
- Add the tests into Widl itself as callable parameter
What do you think is the best solution? Getting some suggestions would be great. :)
There are options. First it depends whether you want to validate against midl or not. If you do then you'll need something like widl.exe to run on Windows.
Next question is what your test data would be, and how to compare the output reliably with expected output. For example we don't really need exact matching for generated headers, or even marshaling format strings, as long as they achieve their purpose. For typelibs however, it might be useful to go for exact binary match of selected sections.
We already have tests for typelibs and marshalling, in oleaut32:typelib and oleaut32:tmarshal respectively. I see no reason that's not sufficient already.
As far as headers are concerned, we could probably take a similar approach in some ways. I don't know what exactly needs testing, but we could do things like
ok(offsetof(mystruct.field) == 16, "wrong offset %u\n", offsetof(mystruct.field));
or (for a compile-time assertion)
extern void test(ITestInterface *obj) { HRESULT (*myfunc)(type1 arg1, type2 *arg2, ...);
myfunc = obj->lpVtbl->myfunc; }
Ultimately we may not need to mess with widl, or write any extra test infrastructure.
I don't agree and I think it'd be good to have widl parsing tests, to validate parser grammar additions, expected parser errors, and the error messages we print if we want to make them better. It will improve the tool quality and help validating any widl change.
If typelibs and other features are already tested elsewhere, maybe we can leave that where it is.
It looks easier to integrate with WineTest and then easier to check the changes (compared to custom scripts) if we're doing it with a PE program or library. I think a set of tests, linked with a static "idlbase" library that implements widl parsing into some IR that the tests would validate, would probably be a simple solution.
I don't think we should necessarily aim for midl byte to byte header compatibility, but it will also make it possible to have a PE widl.exe that could used as a drop-in replacement.
On 6/15/22 13:21, Rémi Bernon wrote:
On 6/15/22 20:04, Zebediah Figura wrote:
On 6/15/22 12:49, Nikolay Sivov wrote:
On 6/15/22 18:20, Bernhard Kölbl wrote:
Hey everyone,
during the course of implementing WinRT / UWP stuff, I regularly come across missing IDL 3.0 features in our Widl compiler. Unfortunately, Widl's code seems to have grown in a rater unpleasant direction and is sometimes a real burden to understand and maintain. I think some refactoring to its code would be very well fitting and make things a lot easier for everyone, but refactoring can also bring breakage and makes tracking down errors with Git harder. Additionally, as the code is already not that small, errors have an easy way to slip in without noticing. So to me, the ideal solution seems to add a testing system to the compiler, like most of Wine's components already have.
Rémi and I already outlined some ideas:
- Port the Winetest system to Unix programs
- Make a libwidl or Widl.exe and test against that with our current
test system
- Add the tests into Widl itself as callable parameter
What do you think is the best solution? Getting some suggestions would be great. :)
There are options. First it depends whether you want to validate against midl or not. If you do then you'll need something like widl.exe to run on Windows.
Next question is what your test data would be, and how to compare the output reliably with expected output. For example we don't really need exact matching for generated headers, or even marshaling format strings, as long as they achieve their purpose. For typelibs however, it might be useful to go for exact binary match of selected sections.
We already have tests for typelibs and marshalling, in oleaut32:typelib and oleaut32:tmarshal respectively. I see no reason that's not sufficient already.
As far as headers are concerned, we could probably take a similar approach in some ways. I don't know what exactly needs testing, but we could do things like
ok(offsetof(mystruct.field) == 16, "wrong offset %u\n", offsetof(mystruct.field));
or (for a compile-time assertion)
extern void test(ITestInterface *obj) { HRESULT (*myfunc)(type1 arg1, type2 *arg2, ...);
myfunc = obj->lpVtbl->myfunc; }
Ultimately we may not need to mess with widl, or write any extra test infrastructure.
I don't agree and I think it'd be good to have widl parsing tests, to validate parser grammar additions, expected parser errors, and the error messages we print if we want to make them better. It will improve the tool quality and help validating any widl change.
I'm not sure what you "don't agree" with, I was just proposing a way to test header contents, and I didn't immediately see any other things that needed testing.
Testing that certain IDLs fail to compile seems reasonable, although checking the exact message strikes me as more restrictive than necessary...
Am Mi., 15. Juni 2022 um 20:04 Uhr schrieb Zebediah Figura zfigura@codeweavers.com:
On 6/15/22 12:49, Nikolay Sivov wrote:
On 6/15/22 18:20, Bernhard Kölbl wrote:
Hey everyone,
during the course of implementing WinRT / UWP stuff, I regularly come across missing IDL 3.0 features in our Widl compiler. Unfortunately, Widl's code seems to have grown in a rater unpleasant direction and is sometimes a real burden to understand and maintain. I think some refactoring to its code would be very well fitting and make things a lot easier for everyone, but refactoring can also bring breakage and makes tracking down errors with Git harder. Additionally, as the code is already not that small, errors have an easy way to slip in without noticing. So to me, the ideal solution seems to add a testing system to the compiler, like most of Wine's components already have.
Rémi and I already outlined some ideas:
- Port the Winetest system to Unix programs
- Make a libwidl or Widl.exe and test against that with our current
test system
- Add the tests into Widl itself as callable parameter
What do you think is the best solution? Getting some suggestions would be great. :)
There are options. First it depends whether you want to validate against midl or not. If you do then you'll need something like widl.exe to run on Windows.
I'd personally not compare against Midl, as we already do some header stuff differently and it also adds a bunch of "useless features".
Next question is what your test data would be, and how to compare the output reliably with expected output. For example we don't really need exact matching for generated headers, or even marshaling format strings, as long as they achieve their purpose. For typelibs however, it might be useful to go for exact binary match of selected sections.
My idea was to compose short snippets of some IDL feature together with their valid output counterparts, and compare if they match after putting the snippet in Widl.
We already have tests for typelibs and marshalling, in oleaut32:typelib and oleaut32:tmarshal respectively. I see no reason that's not sufficient already.
As far as headers are concerned, we could probably take a similar approach in some ways. I don't know what exactly needs testing, but we could do things like
ok(offsetof(mystruct.field) == 16, "wrong offset %u\n", offsetof(mystruct.field));
or (for a compile-time assertion)
extern void test(ITestInterface *obj) { HRESULT (*myfunc)(type1 arg1, type2 *arg2, ...);
myfunc = obj->lpVtbl->myfunc;}
This won't work for some WinRT features like namespaces and some attributes.
Ultimately we may not need to mess with widl, or write any extra test infrastructure.
On 6/15/22 13:24, Bernhard Kölbl wrote:
We already have tests for typelibs and marshalling, in oleaut32:typelib and oleaut32:tmarshal respectively. I see no reason that's not sufficient already.
As far as headers are concerned, we could probably take a similar approach in some ways. I don't know what exactly needs testing, but we could do things like
ok(offsetof(mystruct.field) == 16, "wrong offset %u\n", offsetof(mystruct.field));
or (for a compile-time assertion)
extern void test(ITestInterface *obj) { HRESULT (*myfunc)(type1 arg1, type2 *arg2, ...);
myfunc = obj->lpVtbl->myfunc;}
This won't work for some WinRT features like namespaces and some attributes.
What needs testing there? Is there anything to do aside from validating that a given IDL compiles? That too doesn't seem like it'd require any extra test infrastructure.
Am Mi., 15. Juni 2022 um 20:40 Uhr schrieb Zebediah Figura zfigura@codeweavers.com:
On 6/15/22 13:24, Bernhard Kölbl wrote:
We already have tests for typelibs and marshalling, in oleaut32:typelib and oleaut32:tmarshal respectively. I see no reason that's not sufficient already.
As far as headers are concerned, we could probably take a similar approach in some ways. I don't know what exactly needs testing, but we could do things like
ok(offsetof(mystruct.field) == 16, "wrong offset %u\n", offsetof(mystruct.field));
or (for a compile-time assertion)
extern void test(ITestInterface *obj) { HRESULT (*myfunc)(type1 arg1, type2 *arg2, ...);
myfunc = obj->lpVtbl->myfunc;}
This won't work for some WinRT features like namespaces and some attributes.
What needs testing there? Is there anything to do aside from validating that a given IDL compiles? That too doesn't seem like it'd require any extra test infrastructure.
For example we want to check if Widl compiles interface Windows.Foundation.TypedEventHandler<Windows.ApplicationModel.PackageCatalog*, Windows.ApplicationModel.PackageContentGroupStagingEventArgs*>; to __FITypedEventHandler_2_Windows__CApplicationModel__CPackageCatalog_Windows__CApplicationModel__CPackageContentGroupStagingEventArgs istead of sth like __FITypedEventHandler_1_Windows__CApplicationModel__CPackageCatalog_Windows__CApplicationModel__CPackageContentGroupStagingEventArgs
Sure, it would compile, but we'd lose output matching to Midl on our end.
On 6/15/22 13:52, Bernhard Kölbl wrote:
Am Mi., 15. Juni 2022 um 20:40 Uhr schrieb Zebediah Figura zfigura@codeweavers.com:
On 6/15/22 13:24, Bernhard Kölbl wrote:
We already have tests for typelibs and marshalling, in oleaut32:typelib and oleaut32:tmarshal respectively. I see no reason that's not sufficient already.
As far as headers are concerned, we could probably take a similar approach in some ways. I don't know what exactly needs testing, but we could do things like
ok(offsetof(mystruct.field) == 16, "wrong offset %u\n", offsetof(mystruct.field));
or (for a compile-time assertion)
extern void test(ITestInterface *obj) { HRESULT (*myfunc)(type1 arg1, type2 *arg2, ...);
myfunc = obj->lpVtbl->myfunc;}
This won't work for some WinRT features like namespaces and some attributes.
What needs testing there? Is there anything to do aside from validating that a given IDL compiles? That too doesn't seem like it'd require any extra test infrastructure.
For example we want to check if Widl compiles interface Windows.Foundation.TypedEventHandler<Windows.ApplicationModel.PackageCatalog*, Windows.ApplicationModel.PackageContentGroupStagingEventArgs*>; to __FITypedEventHandler_2_Windows__CApplicationModel__CPackageCatalog_Windows__CApplicationModel__CPackageContentGroupStagingEventArgs istead of sth like __FITypedEventHandler_1_Windows__CApplicationModel__CPackageCatalog_Windows__CApplicationModel__CPackageContentGroupStagingEventArgs
Sure, it would compile, but we'd lose output matching to Midl on our end.
Sure, but you can test that the same way. Just use the symbol somehow and check that it compiles. Am I missing something?
Well we're also missing, that Widl provides valid C++ headers and that'd have to introduce C++ into Wine.
Am Mi., 15. Juni 2022 um 21:08 Uhr schrieb Zebediah Figura zfigura@codeweavers.com:
On 6/15/22 13:52, Bernhard Kölbl wrote:
Am Mi., 15. Juni 2022 um 20:40 Uhr schrieb Zebediah Figura zfigura@codeweavers.com:
On 6/15/22 13:24, Bernhard Kölbl wrote:
We already have tests for typelibs and marshalling, in oleaut32:typelib and oleaut32:tmarshal respectively. I see no reason that's not sufficient already.
As far as headers are concerned, we could probably take a similar approach in some ways. I don't know what exactly needs testing, but we could do things like
ok(offsetof(mystruct.field) == 16, "wrong offset %u\n", offsetof(mystruct.field));
or (for a compile-time assertion)
extern void test(ITestInterface *obj) { HRESULT (*myfunc)(type1 arg1, type2 *arg2, ...);
myfunc = obj->lpVtbl->myfunc;}
This won't work for some WinRT features like namespaces and some attributes.
What needs testing there? Is there anything to do aside from validating that a given IDL compiles? That too doesn't seem like it'd require any extra test infrastructure.
For example we want to check if Widl compiles interface Windows.Foundation.TypedEventHandler<Windows.ApplicationModel.PackageCatalog*, Windows.ApplicationModel.PackageContentGroupStagingEventArgs*>; to __FITypedEventHandler_2_Windows__CApplicationModel__CPackageCatalog_Windows__CApplicationModel__CPackageContentGroupStagingEventArgs istead of sth like __FITypedEventHandler_1_Windows__CApplicationModel__CPackageCatalog_Windows__CApplicationModel__CPackageContentGroupStagingEventArgs
Sure, it would compile, but we'd lose output matching to Midl on our end.
Sure, but you can test that the same way. Just use the symbol somehow and check that it compiles. Am I missing something?
On Wed, Jun 15, 2022 at 10:20 AM Bernhard Kölbl besentv@gmail.com wrote:
- Port the Winetest system to Unix programs
This has the advantage that we can potentially use it to test winex11.drv from the X11 side, which is something that I think we need to make further progress on the X11 driver.
On 6/15/22 17:20, Bernhard Kölbl wrote:
Hey everyone,
during the course of implementing WinRT / UWP stuff, I regularly come across missing IDL 3.0 features in our Widl compiler. Unfortunately, Widl's code seems to have grown in a rater unpleasant direction and is sometimes a real burden to understand and maintain. I think some refactoring to its code would be very well fitting and make things a lot easier for everyone, but refactoring can also bring breakage and makes tracking down errors with Git harder. Additionally, as the code is already not that small, errors have an easy way to slip in without noticing. So to me, the ideal solution seems to add a testing system to the compiler, like most of Wine's components already have.
Rémi and I already outlined some ideas:
- Port the Winetest system to Unix programs
- Make a libwidl or Widl.exe and test against that with our current test system
- Add the tests into Widl itself as callable parameter
What do you think is the best solution? Getting some suggestions would be great. :)
Thanks, Bernhard
Hi, I thought a bit more about this topic.
What we currently have, from the use of IDLs in Wine, as well as from the existing tests, are checks that validate that WIDL output is ABI compatible with MIDL output, but not really source compatible.
What we additionally need in order to make WIDL more robust and changes easier to review, are tests which:
1) Compare what MIDL supports in term of syntax to what we do, to check whether WIDL parsing changes are correct and required.
2) Compare what MIDL generates from a given IDL to what we do, to check whether WIDL code generation changes are correct.
In order to do that I think we actually have to write tests against native MIDL.
I think 1) is the most critical part, and it's actually the easiest. We can simply write some IDL snippets and run them through MIDL, checking its success or failure, and do the same with WIDL.
For 2), it's a bit more tricky, but I think with a bit of post-processing and string matching it should be possible to check whether the generated files have the expected constructs.
I've made a POC implementation of such tests for 1), in the attached patches. They introduces a new builtin midl.exe, that could ultimately support the same command-line options as MIDL and act as a drop-in replacement, as well as associated tests which require the Windows SDKs to be installed to find native midl.exe to run against.
(I'm only not sure that the use of PARENTSRC to share sources between tools/widl and the new programs/midl is the best way to do it.)
Cheers,
I really like the midl.exe idea, as it seems simple to integrate and use with the existing tools.
Thanks, Bernhard
Am Mi., 6. Juli 2022 um 11:48 Uhr schrieb Rémi Bernon rbernon@codeweavers.com:
On 6/15/22 17:20, Bernhard Kölbl wrote:
Hey everyone,
during the course of implementing WinRT / UWP stuff, I regularly come across missing IDL 3.0 features in our Widl compiler. Unfortunately, Widl's code seems to have grown in a rater unpleasant direction and is sometimes a real burden to understand and maintain. I think some refactoring to its code would be very well fitting and make things a lot easier for everyone, but refactoring can also bring breakage and makes tracking down errors with Git harder. Additionally, as the code is already not that small, errors have an easy way to slip in without noticing. So to me, the ideal solution seems to add a testing system to the compiler, like most of Wine's components already have.
Rémi and I already outlined some ideas:
- Port the Winetest system to Unix programs
- Make a libwidl or Widl.exe and test against that with our current test system
- Add the tests into Widl itself as callable parameter
What do you think is the best solution? Getting some suggestions would be great. :)
Thanks, Bernhard
Hi, I thought a bit more about this topic.
What we currently have, from the use of IDLs in Wine, as well as from the existing tests, are checks that validate that WIDL output is ABI compatible with MIDL output, but not really source compatible.
What we additionally need in order to make WIDL more robust and changes easier to review, are tests which:
- Compare what MIDL supports in term of syntax to what we do, to check
whether WIDL parsing changes are correct and required.
- Compare what MIDL generates from a given IDL to what we do, to check
whether WIDL code generation changes are correct.
In order to do that I think we actually have to write tests against native MIDL.
I think 1) is the most critical part, and it's actually the easiest. We can simply write some IDL snippets and run them through MIDL, checking its success or failure, and do the same with WIDL.
For 2), it's a bit more tricky, but I think with a bit of post-processing and string matching it should be possible to check whether the generated files have the expected constructs.
I've made a POC implementation of such tests for 1), in the attached patches. They introduces a new builtin midl.exe, that could ultimately support the same command-line options as MIDL and act as a drop-in replacement, as well as associated tests which require the Windows SDKs to be installed to find native midl.exe to run against.
(I'm only not sure that the use of PARENTSRC to share sources between tools/widl and the new programs/midl is the best way to do it.)
Cheers,
Rémi Bernon rbernon@codeweavers.com
-
Bernhard Kölbl -
Connor McAdams -
Esme Povirk (she/they) -
Nikolay Sivov -
Rémi Bernon -
Zebediah Figura