NordSecurity / uniffi-bindgen-go

Uniffi bindings generator for Golang
Mozilla Public License 2.0
65 stars 18 forks source link

Use pointers to nested variant errors to avoid compilation errors #47

Closed kegsay closed 3 months ago

kegsay commented 3 months ago

Fixes an issue I've been having where the given test case fails with:

generated/errors/errors.go:1399:12: cannot use FfiConverterTypeValidationErrorINSTANCE.Read(reader) (value of type error) as ValidationError value in struct literal: need type assertion
generated/errors/errors.go:1410:57: cannot use variantValue.Source (variable of type ValidationError) as *ValidationError value in argument to FfiConverterTypeValidationErrorINSTANCE.Write

This seems to be because the code sometimes expected *ValidationError and sometimes ValidationError. This patch makes the code use *ValidationError everywhere.

Tests should hopefully prove they don't nil deference.

kegsay commented 3 months ago

The most recent commit does fix #36 by using the same error_type_cast filter on the callback code. I've added the test case to this PR.

EDIT: but this breaks something unrelated, going to back this out for now. This PR does not fix #36 yet.

Savolro commented 3 months ago

Hi and thank you for the contribution! What "unrelated" issues does this cause in particular?

kegsay commented 3 months ago

This PR as it stands is fine and suitable to be merged.

I tried adding the test case in #36 and saw that it didn't actually fix that particular issue because I didn't modify CallbackInterfaceTemplate.go to use error_type_cast. However, when I added this, it caused an unrelated issue where any callback which is an enum type but not an error would fail to compile, producing e.g: generated/errors/errors.go:1780:27: invalid operation: FfiConverterTypeSomeEnumINSTANCE.Read(reader) (value of type SomeEnum) is not an interface. This error happens because we are trying to type cast on something that isn't an interface. This isn't a problem for this PR because by the time we hit ErrorTemplate.go which uses error_type_cast we know that the enum is an error due to this in Types.go:

{%- when Type::Enum { name, module_path } %}
{%- let e = ci.get_enum_definition(name).expect("missing enum") %}
{%- if ci.is_name_used_as_error(name) %}
{%- include "ErrorTemplate.go" %}
{%- else %}
{%- include "EnumTemplate.go" %}
{% endif %}

To fix this for the callback, we need to know if the enum is an error, which means calling ci.is_name_used_as_error(name) which means accessing ComponentInterface inside the GoCodeOracle which doesn't seem possible..

EDIT: I can probably do something like {{arg|error_type_cast(ci.is_name_used_as_error(arg.type_.name()))}}(though this doesn't compile..) and modify error_type_cast to be:

    pub fn error_type_cast(
        type_: &impl AsType,
        is_name_used_as_error: bool,
    )

I could do with some pointers on what the template should look like here. It'll be a separate PR.

kegsay commented 3 months ago

Right, I got this working by calling ci.is_name_used_as_error in the template itself, and passing the bool down to rust. I've created https://github.com/NordSecurity/uniffi-bindgen-go/pull/48 which obsoletes this PR.