Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Error as callback parameter does not compile #36

Closed
arg0d opened this issue Nov 29, 2023 · 3 comments
Closed

Error as callback parameter does not compile #36

arg0d opened this issue Nov 29, 2023 · 3 comments
Labels
bug Something isn't working

Comments

@arg0d
Copy link
Contributor

arg0d commented Nov 29, 2023

Compilation error in generated bindings:

generated/errors/errors.go:1560:23: cannot use FfiConverterTypeBoobyTrapErrorINSTANCE.Read(reader) (value of type error) as type *BoobyTrapError in argument to callback.DoSomething:
	need type assertion

Steps to repro in tests (0.2.0+v0.25.0):

diff --git a/fixtures/errors/src/errors.udl b/fixtures/errors/src/errors.udl
index c8fc765..dcbbf21 100644
--- a/fixtures/errors/src/errors.udl
+++ b/fixtures/errors/src/errors.udl
@@ -35,6 +35,10 @@ interface ComplexError {
     Option(i32? id_a, i32? id_b);
 };
 
+callback interface Callback {
+    void do_something(BoobyTrapError error);
+};
+
 dictionary Vec2 {
     double x;
     double y;
diff --git a/fixtures/errors/src/lib.rs b/fixtures/errors/src/lib.rs
index 7b07383..66c4515 100644
--- a/fixtures/errors/src/lib.rs
+++ b/fixtures/errors/src/lib.rs
@@ -51,6 +51,10 @@ impl Vec2 {
     }
 }
 
+pub trait Callback {
+    fn do_something(&self, error: BoobyTrapError);
+}
+
 fn try_void(trip: bool) -> Result<(), BoobyTrapError> {
     if trip {
         Err(BoobyTrapError::IceSlip)

Source: #35

@kegsay
Copy link
Contributor

kegsay commented Mar 14, 2024

I think I have a failing test case for this in kegsay/uniffi-bindgen-go@main...kegsay:uniffi-bindgen-go:kegan/nested-error - build.sh and build_bindings.sh work fine but on test_bindings:

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

I think the right solution is to take a pointer to ValidationError but I'd like confirmation first from someone e.g @arg0d ?

Specifically, taking a pointer to Source, including in the New.. func:

// Variant structs
type NestedErrorNested struct {
	Source ValidationError // <-- *ValidationError
}

func NewNestedErrorNested(
	source ValidationError, // <-- *ValidationError
) *NestedError {
	return &NestedError{
		err: &NestedErrorNested{
			Source: source,
		},
	}
}

at which point the only compilation failure is in:

		return &NestedError{&NestedErrorNested{
			Source: FfiConverterTypeValidationErrorINSTANCE.Read(reader),
		}}

because .Read returns an error which != *ValidationError, but that can be type cast with .(*ValidationError).

Doing both of these things causes the tests to pass.

@arg0d
Copy link
Contributor Author

arg0d commented Mar 14, 2024

@Savolro maybe you want to take a look at this one? I think you are the one who last touched errors as concrete types for callbacks, instead of error 😉

kegsay added a commit to kegsay/uniffi-bindgen-go that referenced this issue Mar 14, 2024
kegsay added a commit to kegsay/uniffi-bindgen-go that referenced this issue Mar 14, 2024
Should fix NordSecurity#36

Definitely 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 added a commit to kegsay/uniffi-bindgen-go that referenced this issue Mar 14, 2024
Should fix NordSecurity#36

Definitely 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.

Signed-off-by: Kegan Dougal <7190048+kegsay@users.noreply.github.com>
kegsay added a commit to kegsay/uniffi-bindgen-go that referenced this issue Mar 15, 2024
Produces code like this:
```
func (foreignCallbackCallbackInterfaceCallback) InvokeDoSomething(callback Callback, args []byte, outBuf *C.RustBuffer) uniffiCallbackResult {
	reader := bytes.NewReader(args)
	callback.DoSomething(FfiConverterTypeBoobyTrapErrorINSTANCE.Read(reader).(*BoobyTrapError))

	return uniffiCallbackResultSuccess
}
func (foreignCallbackCallbackInterfaceCallback) InvokeDoSomethingElse(callback Callback, args []byte, outBuf *C.RustBuffer) uniffiCallbackResult {
	reader := bytes.NewReader(args)
	callback.DoSomethingElse(FfiConverterTypeSomeEnumINSTANCE.Read(reader))

	return uniffiCallbackResultSuccess
}
```
Note the type-cast on `Read(reader)` for errors.

Signed-off-by: Kegan Dougal <7190048+kegsay@users.noreply.github.com>
kegsay added a commit to kegsay/uniffi-bindgen-go that referenced this issue Mar 20, 2024
Generate pointers to structs consistently, rather than using the
`error` interface in places. This fixes two issues:
 - fix an issue where nested variants did not work correctly when the nested variant was an error.
 - fix an issue where a callback could not be used with an error  (NordSecurity#36)
kegsay added a commit to kegsay/uniffi-bindgen-go that referenced this issue Mar 20, 2024
Generate pointers to structs consistently, rather than using the
`error` interface in places. This fixes two issues:
 - fix an issue where nested variants did not work correctly when the nested variant was an error.
 - fix an issue where a callback could not be used with an error  (NordSecurity#36)

Signed-off-by: Kegan Dougal <7190048+kegsay@users.noreply.github.com>
arg0d pushed a commit that referenced this issue Mar 21, 2024
Generate pointers to structs consistently, rather than using the
`error` interface in places. This fixes two issues:
 - fix an issue where nested variants did not work correctly when the nested variant was an error.
 - fix an issue where a callback could not be used with an error  (#36)

Signed-off-by: Kegan Dougal <7190048+kegsay@users.noreply.github.com>
@kegsay
Copy link
Contributor

kegsay commented Mar 23, 2024

This can be closed now.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants