From 2849809fe4b0f4387a8d44b3872f97fdbf297c1d Mon Sep 17 00:00:00 2001 From: Lili Zoey Zyli Date: Sat, 19 Oct 2024 00:30:20 +0200 Subject: [PATCH] Add `IsBase` macro for better error reporting --- godot-core/src/obj/traits.rs | 15 ++++++ godot-macros/src/class/data_models/field.rs | 8 +-- godot-macros/src/class/derive_godot_class.rs | 57 +++++++------------- 3 files changed, 34 insertions(+), 46 deletions(-) diff --git a/godot-core/src/obj/traits.rs b/godot-core/src/obj/traits.rs index 41ddd1d9d..a47e33128 100644 --- a/godot-core/src/obj/traits.rs +++ b/godot-core/src/obj/traits.rs @@ -76,6 +76,21 @@ impl GodotClass for NoBase { const INIT_LEVEL: InitLevel = InitLevel::Core; // arbitrary; never read. } +#[diagnostic::on_unimplemented( + message = "expected base `{Self}` found `{A}`", + label = "expected base `{Self}` found `{A}`" +)] +#[doc(hidden)] +pub trait IsBase { + #[doc(hidden)] + fn conv(b: Base) -> A; +} +impl IsBase> for Base { + fn conv(b: Base) -> Base { + b + } +} + unsafe impl Bounds for NoBase { type Memory = bounds::MemManual; type DynMemory = bounds::MemManual; diff --git a/godot-macros/src/class/data_models/field.rs b/godot-macros/src/class/data_models/field.rs index b6a05c197..5ba993539 100644 --- a/godot-macros/src/class/data_models/field.rs +++ b/godot-macros/src/class/data_models/field.rs @@ -8,7 +8,6 @@ use crate::class::{FieldExport, FieldVar}; use proc_macro2::{Ident, Span, TokenStream}; use quote::ToTokens; -use venial::Error; pub struct Field { pub name: Ident, @@ -45,16 +44,11 @@ pub struct Fields { /// The field with type `Base`, if available. pub base_field: Option, - /// The base field is either absent or is correctly formatted. - /// - /// When this is false, there will always be a compile error ensuring the program fails to compile. - pub well_formed_base: bool, - /// Deprecation warnings. pub deprecations: Vec, /// Errors during macro evaluation that shouldn't abort the execution of the macro. - pub errors: Vec, + pub errors: Vec, } #[derive(Clone)] diff --git a/godot-macros/src/class/derive_godot_class.rs b/godot-macros/src/class/derive_godot_class.rs index 09c3c5c01..01c7f25e5 100644 --- a/godot-macros/src/class/derive_godot_class.rs +++ b/godot-macros/src/class/derive_godot_class.rs @@ -63,33 +63,21 @@ pub fn derive_godot_class(item: venial::Item) -> ParseResult { let prv = quote! { ::godot::private }; let godot_exports_impl = make_property_impl(class_name, &fields); - let godot_withbase_impl = if let Some(Field { name, .. }) = &fields.base_field { - let implementation = if fields.well_formed_base { - quote! { - fn to_gd(&self) -> ::godot::obj::Gd { - self.#name.to_gd().cast() + let godot_withbase_impl = if let Some(Field { name, ty, .. }) = &fields.base_field { + // Apply the span of the field's type so that errors show up on the field's type. + quote_spanned! { ty.span()=> + impl ::godot::obj::WithBaseField for #class_name { + fn to_gd(&self) -> ::godot::obj::Gd<#class_name> { + // By not referencing the base field directly here we ensure that the user only gets one error when the base + // field's type is wrong. + let base = <#class_name as ::godot::obj::WithBaseField>::base_field(self); + base.to_gd().cast() } - fn base_field(&self) -> &::godot::obj::Base<::Base> { + fn base_field(&self) -> &::godot::obj::Base<<#class_name as ::godot::obj::GodotClass>::Base> { &self.#name } } - } else { - quote! { - fn to_gd(&self) -> ::godot::obj::Gd { - todo!() - } - - fn base_field(&self) -> &::godot::obj::Base<::Base> { - todo!() - } - } - }; - - quote! { - impl ::godot::obj::WithBaseField for #class_name { - #implementation - } } } else { TokenStream::new() @@ -241,14 +229,15 @@ impl ClassAttributes { } fn make_godot_init_impl(class_name: &Ident, fields: &Fields) -> TokenStream { - let base_init = if let Some(Field { name, .. }) = &fields.base_field { - let base = if fields.well_formed_base { - quote! { base } - } else { - quote! { ::std::todo!("The base field is currently broken") } + let base_init = if let Some(Field { name, ty, .. }) = &fields.base_field { + let base_type = + quote_spanned! { ty.span()=> <#class_name as ::godot::obj::GodotClass>::Base }; + let base_field_type = quote_spanned! { ty.span()=> ::godot::obj::Base<#base_type> }; + let base = quote_spanned! { ty.span()=> + <#base_field_type as ::godot::obj::IsBase<#base_type, #ty>>::conv(base) }; - quote! { #name: #base, } + quote_spanned! { ty.span()=> #name: #base, } } else { TokenStream::new() }; @@ -267,9 +256,7 @@ fn make_godot_init_impl(class_name: &Ident, fields: &Fields) -> TokenStream { quote! { impl ::godot::obj::cap::GodotDefault for #class_name { - fn __godot_user_init(base: ::godot::obj::Base) -> Self { - // If the base field is broken then we may get unreachable code due to the `todo`. - #[allow(unreachable_code)] + fn __godot_user_init(base: ::godot::obj::Base<<#class_name as ::godot::obj::GodotClass>::Base>) -> Self { Self { #( #rest_init )* #base_init @@ -451,8 +438,6 @@ fn parse_fields( let mut base_field = Option::::None; let mut deprecations = vec![]; let mut errors = vec![]; - // Base field is either absent or exists and has no errors. - let mut well_formed_base = true; // Attributes on struct fields for (named_field, _punct) in named_fields { @@ -573,7 +558,6 @@ fn parse_fields( // Extra validation; eventually assign to base_fields or all_fields. if is_base { if field.is_onready { - well_formed_base = false; errors.push(error!( field.ty.clone(), "base field cannot have type `OnReady`" @@ -581,7 +565,6 @@ fn parse_fields( } if let Some(var) = field.var.as_ref() { - well_formed_base = false; errors.push(error!( var.span, "base field cannot have the attribute #[var]" @@ -589,7 +572,6 @@ fn parse_fields( } if let Some(export) = field.export.as_ref() { - well_formed_base = false; errors.push(error!( export.span, "base field cannot have the attribute #[export]" @@ -597,7 +579,6 @@ fn parse_fields( } if let Some(default_val) = field.default_val.as_ref() { - well_formed_base = false; errors.push(error!( default_val.span, "base field cannot have the attribute #[init]" @@ -605,7 +586,6 @@ fn parse_fields( } if let Some(prev_base) = base_field.replace(field) { - well_formed_base = false; // Ensure at most one Base. errors.push(error!( // base_field.unwrap().name, @@ -621,7 +601,6 @@ fn parse_fields( Ok(Fields { all_fields, base_field, - well_formed_base, deprecations, errors, })