Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Fixing bug in opaque/struct cleanup codegen in kotlin #690

Merged
merged 3 commits into from
Sep 17, 2024

Conversation

emarteca
Copy link
Contributor

@emarteca emarteca commented Sep 17, 2024

Previously there was a bug where the memory cleanup for multiple pass-by-reference arguments would end up on the same line (note: the cleanup code is only generated if the function in question returns a rust-owned object). This PR fixes it.

Example of bug:
Rust

#[diplomat::bridge]
mod ffi {
    #[diplomat::opaque]
    struct ExampleStruct {
        notempty: bool,
    }

    #[diplomat::opaque]
    struct RustOwnedBytes {
        my_bytes: Vec<u8>,
    }

    impl ExampleStruct {
        pub fn testing(&self, a: &[u8], b: &[u8]) -> Box<RustOwnedBytes> {
            return Box::new(RustOwnedBytes{my_bytes: a.to_vec()})
        }
    }
}

This would generate the following bug in Kotlin (buggy line commented):

 fun testing(a: UByteArray, b: UByteArray): RustOwnedBytes {
        val (aMem, aSlice) = PrimitiveArrayTools.native(a)
        val (bMem, bSlice) = PrimitiveArrayTools.native(b)
        
        val returnVal = lib.ExampleStruct_testing(handle, aSlice, bSlice);
        val selfEdges: List<Any> = listOf()
        val handle = returnVal 
        val returnOpaque = RustOwnedBytes(handle, selfEdges)
        CLEANER.register(returnOpaque, RustOwnedBytes.RustOwnedBytesCleaner(handle, RustOwnedBytes.lib));
        aMem.close()bMem.close() // THIS IS THE BUG THE CLEANUP LINES ARE ON THE SAME LINE
        return returnOpaque
    }

@emarteca emarteca marked this pull request as ready for review September 17, 2024 01:19
@Manishearth Manishearth merged commit 57e8916 into rust-diplomat:main Sep 17, 2024
20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants