Merge godot-rust#109

109: Address sanitizers in integration tests r=Bromeon a=Bromeon

Also makes clippy more strict and fixes doctests on `master`.

Integration tests with address sanitizers currently support two setups (gcc + clang), and are allowed to fail until some of the underlying issues like godot-rust#89 are fixed. Unless the two variants report different findings, we may reduce CI to use only one of the two memory checkers in the future.

Co-authored-by: Jan Haller <[email protected]>

.github/workflows/full-ci.yml

@@ -14,7 +14,7 @@ on:
       - trying
 
 env:
-  GDEXT_FEATURES: 'godot-core/convenience'
+  GDEXT_FEATURES: '--features godot-core/convenience'
 #  GDEXT_CRATE_ARGS: '-p godot-codegen -p godot-ffi -p godot-core -p godot-macros -p godot'
 
 defaults:
@@ -59,7 +59,9 @@ jobs:
           binary-filename: godot.linuxbsd.editor.dev.x86_64
 
       - name: "Check clippy"
-        run: cargo clippy -- -D clippy::style -D clippy::complexity -D clippy::perf -D clippy::dbg_macro -D clippy::todo -D clippy::unimplemented
+        run: |
+          cargo clippy --all-targets $GDEXT_FEATURES -- -D clippy::suspicious -D clippy::style -D clippy::complexity -D clippy::perf \
+          -D clippy::dbg_macro -D clippy::todo -D clippy::unimplemented -D warnings
 
 
   unit-test:
@@ -126,7 +128,7 @@ jobs:
           binary-filename: ${{ matrix.godot-binary }}
 
       - name: "Compile tests"
-        run: cargo test --no-run
+        run: cargo test $GDEXT_FEATURES --no-run
 
       - name: "Test"
         run: cargo test $GDEXT_FEATURES
@@ -135,7 +137,8 @@ jobs:
   godot-itest:
     name: godot-itest (${{ matrix.name }})
     runs-on: ${{ matrix.os }}
-    continue-on-error: false
+    # TODO: continue-on-error: false, as soon as memory errors are fixed
+    continue-on-error: ${{ contains(matrix.name, 'memcheck') }}
     timeout-minutes: 24
     strategy:
       fail-fast: false # cancel all jobs as soon as one fails?
@@ -161,6 +164,16 @@ jobs:
             rust-toolchain: stable
             godot-binary: godot.linuxbsd.editor.dev.x86_64
 
+          - name: linux-memcheck-gcc
+            os: ubuntu-20.04
+            rust-toolchain: stable
+            godot-binary: godot.linuxbsd.editor.dev.x86_64.san
+
+          - name: linux-memcheck-clang
+            os: ubuntu-20.04
+            rust-toolchain: stable
+            godot-binary: godot.linuxbsd.editor.dev.x86_64.llvm.san
+
     steps:
       - uses: actions/checkout@v3
 

.github/workflows/minimal-ci.yml

@@ -14,7 +14,7 @@ on:
       - master
 
 env:
-  GDEXT_FEATURES: 'godot-core/convenience'
+  GDEXT_FEATURES: '--features godot-core/convenience'
 #  GDEXT_CRATE_ARGS: '-p godot-codegen -p godot-ffi -p godot-core -p godot-macros -p godot'
 
 defaults:
@@ -59,7 +59,9 @@ jobs:
           binary-filename: godot.linuxbsd.editor.dev.x86_64
 
       - name: "Check clippy"
-        run: cargo clippy -- -D clippy::style -D clippy::complexity -D clippy::perf -D clippy::dbg_macro -D clippy::todo -D clippy::unimplemented
+        run: |
+          cargo clippy --all-targets $GDEXT_FEATURES -- -D clippy::suspicious -D clippy::style -D clippy::complexity -D clippy::perf \
+          -D clippy::dbg_macro -D clippy::todo -D clippy::unimplemented -D warnings
 
 
   unit-test:

check.sh

@@ -76,7 +76,7 @@ for arg in "${args[@]}"; do
         cmds+=("cargo fmt --all -- --check")
         ;;
     clippy)
-        cmds+=("cargo clippy $features -- -D clippy::style -D clippy::complexity -D clippy::perf -D clippy::dbg_macro -D clippy::todo -D clippy::unimplemented -D warnings")
+        cmds+=("cargo clippy $features -- -D clippy::suspicious -D clippy::style -D clippy::complexity -D clippy::perf -D clippy::dbg_macro -D clippy::todo -D clippy::unimplemented -D warnings")
         ;;
     test)
         cmds+=("cargo test $features")

godot-core/src/builtin/dictionary.rs

@@ -328,15 +328,15 @@ impl Share for Dictionary {
 ///
 /// Example
 /// ```no_run
-/// use godot::builtin::dict;
+/// use godot::builtin::{dict, Variant};
 ///
 /// let key = "my_key";
 /// let d = dict! {
 ///     "key1": 10,
 ///     "another": Variant::nil(),
 ///     key: true,
 ///     (1 + 2): "final",
-/// }
+/// };
 /// ```
 #[macro_export]
 macro_rules! dict {

godot-ffi/src/godot_ffi.rs

@@ -8,12 +8,19 @@ use crate as sys;
 use std::fmt::Debug;
 
 /// Adds methods to convert from and to Godot FFI pointers.
-#[doc(hidden)]
+/// See [crate::ffi_methods] for ergonomic implementation.
 pub trait GodotFfi {
     /// Construct from Godot opaque pointer.
+    ///
+    /// # Safety
+    /// `ptr` must be a valid _type ptr_: it must follow Godot's convention to encode `Self`,
+    /// which is different depending on the type.
     unsafe fn from_sys(ptr: sys::GDExtensionTypePtr) -> Self;
 
-    /// Construct uninitialized opaque data, then initialize it with `init` function.
+    /// Construct uninitialized opaque data, then initialize it with `init_fn` function.
+    ///
+    /// # Safety
+    /// `init_fn` must be a function that correctly handles a (possibly-uninitialized) _type ptr_.
     unsafe fn from_sys_init(init_fn: impl FnOnce(sys::GDExtensionTypePtr)) -> Self;
 
     /// Return Godot opaque pointer, for an immutable operation.
@@ -38,6 +45,11 @@ pub trait GodotFfi {
         self.sys()
     }
 
+    /// Write the contents of `self` into the pointer `dst`.
+    ///
+    /// # Safety
+    /// `dst` must be a valid _type ptr_: it must follow Godot's convention to encode `Self`,
+    /// which is different depending on the type.
     unsafe fn write_sys(&self, dst: sys::GDExtensionTypePtr);
 }
 
@@ -66,6 +78,7 @@ pub trait GodotFuncMarshal: Sized {
 // See doc comment of `ffi_methods!` for information
 
 #[macro_export]
+#[doc(hidden)]
 macro_rules! ffi_methods_one {
 	// type $Ptr = *mut Opaque
  	(OpaquePtr $Ptr:ty; $( #[$attr:meta] )? $vis:vis $from_sys:ident = from_sys) => {
@@ -159,6 +172,7 @@ macro_rules! ffi_methods_one {
 }
 
 #[macro_export]
+#[doc(hidden)]
 macro_rules! ffi_methods_rest {
 	( // impl T: each method has a custom name and is annotated with 'pub'
 		$Impl:ident $Ptr:ty; $( fn $user_fn:ident = $sys_fn:ident; )*

godot/src/lib.rs

@@ -99,6 +99,7 @@ pub use godot_core::private;
 /// Often-imported symbols.
 pub mod prelude {
     pub use super::bind::{godot_api, GodotClass, GodotExt};
+    pub use super::builtin::dict; // Re-export macros.
     pub use super::builtin::*;
     pub use super::engine::{
         load, try_load, utilities, AudioStreamPlayer, Camera2D, Camera3D, Input, Node, Node2D,