[PATCH v2 0/4] MR8500: Some improvements to widl.
This MR improves widl with : - fixes a segfault when inheriting from an incomplete interface (prints an error message instead), - supports (for C++) inheriting from interfaces which are declared after the declaration of the derived interface. -- v2: include: Remove duplicated declarations in .idl files. tools/widl: Ensure inherited interface is declared before using it. tools/widl: Remove unneeded condition. tools/widl: Fix segfault when inheriting from an incomplete interface. https://gitlab.winehq.org/wine/wine/-/merge_requests/8500
From: Eric Pouech <epouech(a)codeweavers.com> Widl segfaults in .idl header translation when inherited interface is just a forward declaration (or when the intended declaration exists but with a typo in its name). Print an error message instead. Signed-off-by: Eric Pouech <epouech(a)codeweavers.com> --- tools/widl/parser.y | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tools/widl/parser.y b/tools/widl/parser.y index 54b143637cf..51856152296 100644 --- a/tools/widl/parser.y +++ b/tools/widl/parser.y @@ -2826,6 +2826,11 @@ static void check_all_user_types(const statement_list_t *stmts) !is_local(stmt->u.type->attrs)) { const statement_t *stmt_func; + const type_t *type = stmt->u.type; + if (type->details.iface && type->details.iface->inherit && !type_is_complete(type->details.iface->inherit)) + error_at(&type->where, "interface %s can't inherit from incomplete interface %s\n", + type->name, type->details.iface->inherit->name); + STATEMENTS_FOR_EACH_FUNC(stmt_func, type_iface_get_stmts(stmt->u.type)) { const var_t *func = stmt_func->u.var; if (type_function_get_args(func->declspec.type)) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8500
From: Eric Pouech <epouech(a)codeweavers.com> 'written' bit is never set for an interface, so the RHS of the test can be removed. (Need for subsequent patch that will set the 'written' bit). Signed-off-by: Eric Pouech <epouech(a)codeweavers.com> --- tools/widl/header.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/widl/header.c b/tools/widl/header.c index e2ce883b788..29c6c32c8f4 100644 --- a/tools/widl/header.c +++ b/tools/widl/header.c @@ -2028,7 +2028,7 @@ static void write_header_stmts(FILE *header, const statement_list_t *stmts, cons case STMT_TYPEREF: /* FIXME: shouldn't write out forward declarations for undefined * interfaces but a number of our IDL files depend on this */ - if (type_get_type(stmt->u.type) == TYPE_INTERFACE && !stmt->u.type->written) + if (type_get_type(stmt->u.type) == TYPE_INTERFACE) write_forward(header, stmt->u.type); break; case STMT_IMPORTLIB: -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8500
From: Eric Pouech <epouech(a)codeweavers.com> If interface I1 inherits from interface I2, midl allows I2 declaration to appear after I1's declaration in .idl file. Keeping the .idl order is fine with C binding, but doesn't work for C++ classes (class I2 must be declared before being inherited from). So ensure inherited interface is declared in header generation before using it. Signed-off-by: Eric Pouech <epouech(a)codeweavers.com> --- tools/widl/header.c | 24 +++++++++++++++++++++--- 1 file changed, 21 insertions(+), 3 deletions(-) diff --git a/tools/widl/header.c b/tools/widl/header.c index 29c6c32c8f4..f20682f2848 100644 --- a/tools/widl/header.c +++ b/tools/widl/header.c @@ -1977,6 +1977,26 @@ static void write_forward_decls(FILE *header, const statement_list_t *stmts) } } +static void write_header_stmts( FILE *header, const statement_list_t *stmts, const type_t *iface, int ignore_funcs ); + +static void write_header_com_interface( FILE *header, type_t *iface, const type_t *ref_iface ) +{ + type_t *inherit_from; + + if (iface->written) return; + + /* ensure declaration of inherited interface exists before ours (C++ requires this) */ + inherit_from = type_iface_get_inherit( iface ); + if (inherit_from && !inherit_from->ignore && type_iface_get_inherit( inherit_from ) /* skip IUnknown */) + write_header_com_interface( header, inherit_from, inherit_from ); + + write_com_interface_start( header, iface ); + write_header_stmts( header, type_iface_get_stmts(iface), ref_iface, TRUE ); + write_com_interface_end( header, iface ); + + iface->written = true; +} + static void write_header_stmts(FILE *header, const statement_list_t *stmts, const type_t *iface, int ignore_funcs) { const statement_t *stmt; @@ -1993,9 +2013,7 @@ static void write_header_stmts(FILE *header, const statement_list_t *stmts, cons if (is_object(iface)) is_object_interface++; if (is_attr(stmt->u.type->attrs, ATTR_DISPINTERFACE) || is_object(stmt->u.type)) { - write_com_interface_start(header, iface); - write_header_stmts(header, type_iface_get_stmts(iface), stmt->u.type, TRUE); - write_com_interface_end(header, iface); + write_header_com_interface(header, iface, stmt->u.type); if (async_iface) { write_com_interface_start(header, async_iface); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8500
From: Eric Pouech <epouech(a)codeweavers.com> Signed-off-by: Eric Pouech <epouech(a)codeweavers.com> --- include/windows.foundation.idl | 1 - include/windows.networking.connectivity.idl | 1 - 2 files changed, 2 deletions(-) diff --git a/include/windows.foundation.idl b/include/windows.foundation.idl index 9e3b656d8a3..5d086623bf4 100644 --- a/include/windows.foundation.idl +++ b/include/windows.foundation.idl @@ -142,7 +142,6 @@ namespace Windows.Foundation { interface Windows.Foundation.IAsyncOperation<HSTRING>; interface Windows.Foundation.IAsyncOperation<IInspectable *>; interface Windows.Foundation.IAsyncOperation<boolean>; - interface Windows.Foundation.IAsyncOperation<HSTRING>; interface Windows.Foundation.IAsyncOperation<UINT32>; #ifndef _WINTYPES interface Windows.Foundation.IAsyncOperation<Windows.Foundation.Uri *>; diff --git a/include/windows.networking.connectivity.idl b/include/windows.networking.connectivity.idl index e452b2e3289..48800b97fb5 100644 --- a/include/windows.networking.connectivity.idl +++ b/include/windows.networking.connectivity.idl @@ -95,7 +95,6 @@ namespace Windows.Networking.Connectivity interface Windows.Foundation.Collections.IVectorView<Windows.Networking.Connectivity.ConnectivityInterval*>; interface Windows.Foundation.IAsyncOperation<Windows.Foundation.Collections.IVectorView<Windows.Networking.Connectivity.ConnectivityInterval*>*>; interface Windows.Foundation.AsyncOperationCompletedHandler<Windows.Foundation.Collections.IVectorView<Windows.Networking.Connectivity.ConnectivityInterval*>*>; - interface Windows.Foundation.AsyncOperationCompletedHandler<Windows.Networking.Connectivity.ConnectionProfile*>; interface Windows.Foundation.AsyncOperationCompletedHandler<Windows.Foundation.Collections.IVectorView<Windows.Networking.Connectivity.NetworkUsage*>*>; } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8500
V2 pushed: * should cover Rémi's comments * with the MR applied, the only differences in all generated .h files stem for two .idl files which have duplicate interface declaration (which we emitted twice before, and only once with this MR applied); fix supplied for the relevant .idl files * diff from V1: ignore the interfaces with the 'ignore' bit set -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8500#note_108801
Rémi Bernon (@rbernon) commented about tools/widl/header.c:
} }
+static void write_header_stmts( FILE *header, const statement_list_t *stmts, const type_t *iface, int ignore_funcs ); + +static void write_header_com_interface( FILE *header, type_t *iface, const type_t *ref_iface ) +{ + type_t *inherit_from; + + if (iface->written) return; + + /* ensure declaration of inherited interface exists before ours (C++ requires this) */ + inherit_from = type_iface_get_inherit( iface ); + if (inherit_from && !inherit_from->ignore && type_iface_get_inherit( inherit_from ) /* skip IUnknown */) Why should we skip interfaces without base?
```suggestion:-0+0 if (inherit_from && !inherit_from->ignore) ``` -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8500#note_108808
participants (3)
-
Eric Pouech -
eric pouech (＠epo)
-
Rémi Bernon