From bcad368a4d3ab4a6f704b0aa813b12e18ec53277 Mon Sep 17 00:00:00 2001 From: Jan Haller Date: Sat, 4 Feb 2023 14:58:17 +0100 Subject: [PATCH 1/3] Elevate `clippy::suspicious` and `warnings` (both default warn) to deny level --- .github/workflows/full-ci.yml | 7 ++++--- .github/workflows/minimal-ci.yml | 6 ++++-- check.sh | 2 +- 3 files changed, 9 insertions(+), 6 deletions(-) diff --git a/.github/workflows/full-ci.yml b/.github/workflows/full-ci.yml index 789a5f3ca..5d83d10ec 100644 --- a/.github/workflows/full-ci.yml +++ b/.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,8 +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: name: unit-test (${{ matrix.name }}${{ matrix.rust-special }}) diff --git a/.github/workflows/minimal-ci.yml b/.github/workflows/minimal-ci.yml index 0997e9724..4646248ce 100644 --- a/.github/workflows/minimal-ci.yml +++ b/.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: diff --git a/check.sh b/check.sh index 69cbdb309..e7aed976b 100755 --- a/check.sh +++ b/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") From 5ea54479876cccfc68030cca654a63f6ca813011 Mon Sep 17 00:00:00 2001 From: Jan Haller Date: Sat, 4 Feb 2023 14:58:59 +0100 Subject: [PATCH 2/3] Smaller fixes for clippy and doctest --- godot-core/src/builtin/dictionary.rs | 4 ++-- godot-ffi/src/godot_ffi.rs | 18 ++++++++++++++++-- godot/src/lib.rs | 1 + 3 files changed, 19 insertions(+), 4 deletions(-) diff --git a/godot-core/src/builtin/dictionary.rs b/godot-core/src/builtin/dictionary.rs index ddc05fba7..273d1829b 100644 --- a/godot-core/src/builtin/dictionary.rs +++ b/godot-core/src/builtin/dictionary.rs @@ -328,7 +328,7 @@ impl Share for Dictionary { /// /// Example /// ```no_run -/// use godot::builtin::dict; +/// use godot::builtin::{dict, Variant}; /// /// let key = "my_key"; /// let d = dict! { @@ -336,7 +336,7 @@ impl Share for Dictionary { /// "another": Variant::nil(), /// key: true, /// (1 + 2): "final", -/// } +/// }; /// ``` #[macro_export] macro_rules! dict { diff --git a/godot-ffi/src/godot_ffi.rs b/godot-ffi/src/godot_ffi.rs index 96effeaab..6d3449860 100644 --- a/godot-ffi/src/godot_ffi.rs +++ b/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; )* diff --git a/godot/src/lib.rs b/godot/src/lib.rs index 94e46fbac..16207cf68 100644 --- a/godot/src/lib.rs +++ b/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, From 93b82010b6a00d0df28b722693cc9a83b733e5dc Mon Sep 17 00:00:00 2001 From: Jan Haller Date: Sat, 4 Feb 2023 15:07:12 +0100 Subject: [PATCH 3/3] Add integration tests for Godot with address sanitizers Currently allowed to fail, until memory errors are fixed. --- .github/workflows/full-ci.yml | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/.github/workflows/full-ci.yml b/.github/workflows/full-ci.yml index 5d83d10ec..6650769fe 100644 --- a/.github/workflows/full-ci.yml +++ b/.github/workflows/full-ci.yml @@ -63,6 +63,7 @@ jobs: 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: name: unit-test (${{ matrix.name }}${{ matrix.rust-special }}) runs-on: ${{ matrix.os }} @@ -127,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 @@ -136,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? @@ -162,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