Add error handling (#49)

Author: emgre

.github/workflows/ci.yml

@@ -16,78 +16,119 @@ jobs:
           - stable
           - beta
           - nightly
-        toolchain:
-          - default
-          - musl
-        exclude:
-          - os: windows-latest
-            toolchain: musl
     runs-on: ${{ matrix.os }}
     steps:
       - name: Checkout
         uses: actions/checkout@v1
       - name: Install Rust ${{ matrix.rust }}
-        if: ${{ matrix.toolchain != 'musl' }}
         uses: actions-rs/toolchain@v1
         with:
           profile: minimal
           toolchain: ${{ matrix.rust }}
           override: true
-      - name: Install Rust ${{ matrix.rust }} (musl)
-        if: ${{ matrix.toolchain == 'musl' }}
+      - name: Copy Windows config
+        if: ${{ runner.os == 'Windows' }}
+        shell: pwsh
+        run: Copy-Item -Path .cargo/config-windows.toml -Destination .cargo/config.toml
+      - name: Copy Linux config
+        if: ${{ runner.os == 'Linux' }}
+        shell: pwsh
+        run: Copy-Item -Path .cargo/config-linux.toml -Destination .cargo/config.toml
+      - name: Caching
+        uses: Swatinem/rust-cache@v1
+      - name: Run Rust unit tests
+        uses: actions-rs/cargo@v1
+        with:
+          command: test
+  linting:
+    strategy:
+      fail-fast: false
+      matrix:
+        rust:
+          - stable
+          - beta
+    runs-on: ubuntu-latest
+    steps:
+      - name: Checkout
+        uses: actions/checkout@v1
+      - name: Install Rust ${{ matrix.rust }}
         uses: actions-rs/toolchain@v1
         with:
           profile: minimal
           toolchain: ${{ matrix.rust }}
-          target: x86_64-unknown-linux-musl
+          override: true
+          components: rustfmt, clippy
+      - name: Caching
+        uses: Swatinem/rust-cache@v1
+      - name: Format
+        uses: actions-rs/cargo@v1
+        with:
+          command: fmt
+          args: --all -- --check
+      - name: Clippy
+        uses: actions-rs/cargo@v1
+        with:
+          command: clippy
+          args: -- -D warnings
+  bindings:
+    strategy:
+      fail-fast: false
+      matrix:
+        os:
+          - ubuntu-latest
+          - windows-latest
+    runs-on: ${{ matrix.os }}
+    steps:
+      - name: Checkout
+        uses: actions/checkout@v1
+      - name: Install Rust
+        uses: actions-rs/toolchain@v1
+        with:
+          profile: minimal
+          toolchain: stable
           override: true
       - name: Copy Windows config
         if: ${{ runner.os == 'Windows' }}
         shell: pwsh
         run: Copy-Item -Path .cargo/config-windows.toml -Destination .cargo/config.toml
       - name: Copy Linux config
-        if: ${{ runner.os == 'Linux' && matrix.toolchain == 'default' }}
+        if: ${{ runner.os == 'Linux' }}
         shell: pwsh
         run: Copy-Item -Path .cargo/config-linux.toml -Destination .cargo/config.toml
-      - name: Copy Linux musl config
-        if: ${{ runner.os == 'Linux' && matrix.toolchain == 'musl' }}
-        shell: pwsh
-        run: Copy-Item -Path .cargo/config-linux-musl.toml -Destination .cargo/config.toml
-      - name: Run Rust unit tests
+      - name: Caching
+        uses: Swatinem/rust-cache@v1
+      - name: Build
         uses: actions-rs/cargo@v1
         with:
           command: test
+          args: --release
       - name: C bindings
         uses: actions-rs/cargo@v1
         with:
           command: run
           args: --release --bin foo-bindings -- --c
       - name: .NET bindings
-        if: ${{ matrix.toolchain != 'musl' }}
         uses: actions-rs/cargo@v1
         with:
           command: run
           args: --release --bin foo-bindings -- --dotnet
       - name: Java bindings
-        if: ${{ matrix.toolchain != 'musl' }}
         uses: actions-rs/cargo@v1
         with:
           command: run
           args: --release --bin foo-bindings -- --java
-      - name: Upload compiled bindings
-        if: ${{ matrix.rust == 'stable' }}
+      - name: Upload compiled C bindings
         uses: actions/upload-artifact@v2
         with:
           name: ffi-modules
           path: tests/bindings/c/generated/lib
       - name: Upload compiled Java bindings
-        if: ${{ matrix.rust == 'stable' && matrix.toolchain != 'musl' }}
         uses: actions/upload-artifact@v2
         with:
           name: ffi-modules
           path: tests/bindings/java/foo/src/main/resources
   packaging:
-    needs: test
+    needs: bindings
     runs-on: ubuntu-latest
     steps:
       - name: Checkout
@@ -98,6 +139,8 @@ jobs:
           profile: minimal
           toolchain: stable
           override: true
+      - name: Caching
+        uses: Swatinem/rust-cache@v1
       - name: Download compiled FFI
         uses: actions/download-artifact@v2
         with:
@@ -107,7 +150,7 @@ jobs:
         uses: actions-rs/cargo@v1
         with:
           command: run
-          args: --release --bin foo-bindings -- --package ./ffi-modules
+          args: --bin foo-bindings -- --package ./ffi-modules
       - name: Upload C bindings
         uses: actions/upload-artifact@v2
         with:
@@ -123,32 +166,3 @@ jobs:
         with:
           name: java-bindings
           path: tests/bindings/java/foo/target/*.jar
-  linting:
-    strategy:
-      fail-fast: false
-      matrix:
-        rust:
-          - stable
-          - beta
-          - nightly
-    runs-on: ubuntu-latest
-    steps:
-      - name: Checkout
-        uses: actions/checkout@v1
-      - name: Install Rust ${{ matrix.rust }}
-        uses: actions-rs/toolchain@v1
-        with:
-          profile: minimal
-          toolchain: ${{ matrix.rust }}
-          override: true
-          components: rustfmt, clippy
-      - name: Format
-        uses: actions-rs/cargo@v1
-        with:
-          command: fmt
-          args: --all -- --check
-      - name: Clippy
-        uses: actions-rs/cargo@v1
-        with:
-          command: clippy
-          args: -- -D warnings

ci-script/src/lib.rs

@@ -52,20 +52,21 @@ pub fn run(settings: BindingBuilderSettings) {
     let run_java = matches.is_present("java");
     let run_all = !run_c && !run_dotnet && !run_java;
 
+    let package = matches.is_present("package");
     let package_src = matches.value_of("package");
 
     if run_c || run_all {
-        let builder = run_builder::<CBindingBuilder>(&settings, run_tests, package_src);
+        let builder = run_builder::<CBindingBuilder>(&settings, run_tests, package, package_src);
 
         if matches.is_present("doxygen") {
             builder.build_doxygen();
         }
     }
     if run_dotnet || run_all {
-        run_builder::<DotnetBindingBuilder>(&settings, run_tests, package_src);
+        run_builder::<DotnetBindingBuilder>(&settings, run_tests, package, package_src);
     }
     if run_java || run_all {
-        run_builder::<JavaBindingBuilder>(&settings, run_tests, package_src);
+        run_builder::<JavaBindingBuilder>(&settings, run_tests, package, package_src);
     }
 }
 
@@ -76,6 +77,7 @@ fn ffi_path() -> PathBuf {
 fn run_builder<'a, B: BindingBuilder<'a>>(
     settings: &'a BindingBuilderSettings,
     run_tests: bool,
+    package: bool,
     package_src: Option<&str>,
 ) -> B {
     let mut platforms = PlatformLocations::new();
@@ -120,9 +122,9 @@ fn run_builder<'a, B: BindingBuilder<'a>>(
         return builder;
     }
 
-    builder.generate(package_src.is_some());
+    builder.generate(package);
 
-    if package_src.is_none() {
+    if !package {
         builder.build();
         if run_tests {
             builder.test();
@@ -333,12 +335,13 @@ impl<'a> BindingBuilder<'a> for DotnetBindingBuilder<'a> {
     }
 
     fn package(&mut self) {
-        // Run unit tests
+        // Produce a nupkg
         let result = Command::new("dotnet")
             .current_dir(&self.output_dir())
             .arg("pack")
             .arg("--configuration")
             .arg("Release")
+            .arg("--include-symbols")
             .arg("--output")
             .arg("nupkg")
             .status()

generators/c-oo-bindgen/src/lib.rs

@@ -113,6 +113,7 @@ impl CFormatting for Symbol {
             Symbol::Struct(handle) => handle.declaration().to_c_type(),
             Symbol::Enum(handle) => handle.to_c_type(),
             Symbol::Class(handle) => handle.declaration().to_c_type(),
+            Symbol::StaticClass(_) => panic!("static classes cannot be referenced in C"),
             Symbol::Interface(handle) => handle.to_c_type(),
             Symbol::OneTimeCallback(handle) => handle.to_c_type(),
             Symbol::Iterator(handle) => handle.iter_type.to_c_type(),
@@ -577,7 +578,7 @@ fn write_class_declaration(
     ))
 }
 
-fn write_function(
+fn write_function_docs(
     f: &mut dyn Printer,
     handle: &NativeFunctionHandle,
     lib: &Library,
@@ -592,22 +593,50 @@ fn write_function(
             docstring_print(f, &param.doc, lib)?;
         }
 
-        // Print return documentation
-        if let ReturnType::Type(_, doc) = &handle.return_type {
-            f.writeln("@return ")?;
-            docstring_print(f, doc, lib)?;
+        fn write_error_return_doc(f: &mut dyn Printer) -> FormattingResult<()> {
+            f.writeln("@return Error code")
+        }
+
+        match handle.get_type() {
+            NativeFunctionType::NoErrorNoReturn => {}
+            NativeFunctionType::NoErrorWithReturn(_, doc) => {
+                f.writeln("@return ")?;
+                docstring_print(f, &doc, lib)?;
+            }
+            NativeFunctionType::ErrorNoReturn(_) => {
+                write_error_return_doc(f)?;
+            }
+            NativeFunctionType::ErrorWithReturn(_, _, doc) => {
+                f.writeln("@param out ")?;
+                docstring_print(f, &doc, lib)?;
+                write_error_return_doc(f)?;
+            }
         }
 
         Ok(())
-    })?;
+    })
+}
 
-    f.newline()?;
+fn write_function(
+    f: &mut dyn Printer,
+    handle: &NativeFunctionHandle,
+    lib: &Library,
+) -> FormattingResult<()> {
+    write_function_docs(f, handle, lib)?;
 
-    f.write(&format!(
-        "{} {}(",
-        CReturnType(&handle.return_type),
-        handle.name
-    ))?;
+    if let Some(error_type) = &handle.error_type {
+        f.writeln(&format!(
+            "{} {}(",
+            CType(&Type::Enum(error_type.inner.clone())),
+            handle.name
+        ))?;
+    } else {
+        f.writeln(&format!(
+            "{} {}(",
+            CReturnType(&handle.return_type),
+            handle.name
+        ))?;
+    }
 
     f.write(
         &handle
@@ -624,6 +653,15 @@ fn write_function(
             .join(", "),
     )?;
 
+    if handle.error_type.is_some() {
+        if let ReturnType::Type(x, _) = &handle.return_type {
+            if !handle.parameters.is_empty() {
+                f.write(", ")?;
+                f.write(&format!("{}* out", CType(x)))?;
+            }
+        }
+    }
+
     f.write(");")
 }
 

generators/dotnet-oo-bindgen/copy/Helpers.cs

@@ -0,0 +1,35 @@
+using System;
+using System.Runtime.InteropServices;
+
+namespace Helpers
+{
+    internal static class RustString
+    {
+        internal static IntPtr ToNative(string value)
+        {
+            var bytes = System.Text.Encoding.UTF8.GetBytes(value);
+            var handle = Marshal.AllocHGlobal(bytes.Length + 1);
+            // copy the bytes of the string
+            Marshal.Copy(bytes, 0, handle, bytes.Length);
+            // null terminator
+            Marshal.WriteByte(handle, bytes.Length, 0);
+            return handle;
+        }
+
+        internal static void Destroy(IntPtr value)
+        {
+            Marshal.FreeHGlobal(value);
+        }
+
+        internal static string FromNative(IntPtr value)
+        {
+            // figure out the length of the string by looking for the NULL terminator
+            int length = 0;
+            while (Marshal.ReadByte(value, length) != 0) ++length;
+            byte[] buffer = new byte[length];
+            // copy from the native type into the byte buffer
+            Marshal.Copy(value, buffer, 0, length);
+            return System.Text.Encoding.UTF8.GetString(buffer);
+        }
+    }
+}