Merge pull request #1418 from nicholasbishop/bishop-macros-cleanup

uefi-macros: Require that the entry function takes zero args

Author: phip1611

uefi-macros/CHANGELOG.md

@@ -1,5 +1,9 @@
 # uefi-macros - [Unreleased]
 
+## Changed
+
+- **Breaking:** The `entry` no longer accepts any arguments.
+
 
 # uefi-macros - 0.16.0 (2024-09-09)
 

uefi-macros/src/lib.rs

@@ -8,8 +8,8 @@ use proc_macro2::TokenStream as TokenStream2;
 use quote::{quote, quote_spanned, TokenStreamExt};
 use syn::spanned::Spanned;
 use syn::{
-    parse_macro_input, parse_quote, parse_quote_spanned, Error, Expr, ExprLit, ExprPath, FnArg,
-    Ident, ItemFn, ItemStruct, Lit, Pat, Visibility,
+    parse_macro_input, parse_quote, parse_quote_spanned, Error, Expr, ExprLit, ExprPath, ItemFn,
+    ItemStruct, Lit, Visibility,
 };
 
 macro_rules! err {
@@ -93,49 +93,19 @@ pub fn unsafe_protocol(args: TokenStream, input: TokenStream) -> TokenStream {
     .into()
 }
 
-/// Get the name of a function's argument at `arg_index`.
-fn get_function_arg_name(f: &ItemFn, arg_index: usize, errors: &mut TokenStream2) -> Option<Ident> {
-    if let Some(FnArg::Typed(arg)) = f.sig.inputs.iter().nth(arg_index) {
-        if let Pat::Ident(pat_ident) = &*arg.pat {
-            // The argument has a valid name such as `handle` or `_handle`.
-            Some(pat_ident.ident.clone())
-        } else {
-            // The argument is unnamed, i.e. `_`.
-            errors.append_all(err!(
-                arg.pat.span(),
-                "Entry method's arguments must be named"
-            ));
-            None
-        }
-    } else {
-        // Either there are too few arguments, or it's the wrong kind of
-        // argument (e.g. `self`).
-        //
-        // Don't append an error in this case. The error will be caught
-        // by the typecheck later on, which will give a better error
-        // message.
-        None
-    }
-}
-
 /// Custom attribute for a UEFI executable entry point.
 ///
 /// This attribute modifies a function to mark it as the entry point for
 /// a UEFI executable. The function:
 /// * Must return [`Status`].
-/// * Must have either zero parameters or two: [`Handle`] and [`SystemTable<Boot>`].
+/// * Must have zero parameters.
 /// * Can optionally be `unsafe`.
 ///
-/// Due to internal implementation details the parameters must both be
-/// named, so `arg` or `_arg` are allowed, but not `_`.
-///
 /// The global system table pointer and global image handle will be set
 /// automatically.
 ///
 /// # Examples
 ///
-/// With no arguments:
-///
 /// ```no_run
 /// #![no_main]
 ///
@@ -147,23 +117,7 @@ fn get_function_arg_name(f: &ItemFn, arg_index: usize, errors: &mut TokenStream2
 /// }
 /// ```
 ///
-/// With two arguments:
-///
-/// ```no_run
-/// #![no_main]
-///
-/// use uefi::prelude::*;
-///
-/// #[entry]
-/// fn main(image: Handle, st: SystemTable<Boot>) -> Status {
-///     Status::SUCCESS
-/// }
-/// ```
-///
-/// [`Handle`]: https://docs.rs/uefi/latest/uefi/data_types/struct.Handle.html
-/// [`SystemTable<Boot>`]: https://docs.rs/uefi/latest/uefi/table/struct.SystemTable.html
 /// [`Status`]: https://docs.rs/uefi/latest/uefi/struct.Status.html
-/// [`BootServices::set_image_handle`]: https://docs.rs/uefi/latest/uefi/table/boot/struct.BootServices.html#method.set_image_handle
 #[proc_macro_attribute]
 pub fn entry(args: TokenStream, input: TokenStream) -> TokenStream {
     // This code is inspired by the approach in this embedded Rust crate:
@@ -181,92 +135,64 @@ pub fn entry(args: TokenStream, input: TokenStream) -> TokenStream {
     let mut f = parse_macro_input!(input as ItemFn);
 
     if let Some(ref abi) = f.sig.abi {
-        errors.append_all(err!(abi, "Entry method must have no ABI modifier"));
+        errors.append_all(err!(abi, "Entry function must have no ABI modifier"));
     }
     if let Some(asyncness) = f.sig.asyncness {
-        errors.append_all(err!(asyncness, "Entry method should not be async"));
+        errors.append_all(err!(asyncness, "Entry function should not be async"));
     }
     if let Some(constness) = f.sig.constness {
-        errors.append_all(err!(constness, "Entry method should not be const"));
+        errors.append_all(err!(constness, "Entry function should not be const"));
     }
     if !f.sig.generics.params.is_empty() {
         errors.append_all(err!(
             f.sig.generics.params,
-            "Entry method should not be generic"
+            "Entry function should not be generic"
         ));
     }
-
-    let signature_span = f.sig.span();
-
-    // If the user doesn't specify any arguments to the entry function, fill in
-    // the image handle and system table arguments automatically.
-    let generated_args = f.sig.inputs.is_empty();
-    if generated_args {
-        f.sig.inputs = parse_quote_spanned!(
-            signature_span=>
-                internal_image_handle: ::uefi::Handle,
-                internal_system_table: *const ::core::ffi::c_void,
-        );
+    if !f.sig.inputs.is_empty() {
+        errors.append_all(err!(f.sig.inputs, "Entry function must have no arguments"));
     }
 
-    let image_handle_ident = get_function_arg_name(&f, 0, &mut errors);
-    let system_table_ident = get_function_arg_name(&f, 1, &mut errors);
-
-    // show most errors at once instead of one by one
+    // Show most errors all at once instead of one by one.
     if !errors.is_empty() {
         return errors.into();
     }
 
-    f.sig.abi = Some(syn::parse2(quote_spanned! (signature_span=> extern "efiapi")).unwrap());
+    let signature_span = f.sig.span();
 
-    // allow the entry function to be unsafe (by moving the keyword around so that it actually works)
-    let unsafety = &f.sig.unsafety;
-    // strip any visibility modifiers
+    // Fill in the image handle and system table arguments automatically.
+    let image_handle_ident = quote!(internal_image_handle);
+    let system_table_ident = quote!(internal_system_table);
+    f.sig.inputs = parse_quote_spanned!(
+        signature_span=>
+            #image_handle_ident: ::uefi::Handle,
+            #system_table_ident: *const ::core::ffi::c_void,
+    );
+
+    // Insert code at the beginning of the entry function to set the global
+    // image handle and system table pointer.
+    f.block.stmts.insert(
+        0,
+        parse_quote! {
+            unsafe {
+                ::uefi::boot::set_image_handle(#image_handle_ident);
+                ::uefi::table::set_system_table(#system_table_ident.cast());
+            }
+        },
+    );
+
+    // Set the required ABI.
+    f.sig.abi = Some(parse_quote_spanned!(signature_span=> extern "efiapi"));
+
+    // Strip any visibility modifiers.
     f.vis = Visibility::Inherited;
-    // Set the global image handle. If `image_handle_ident` is `None`
-    // then the typecheck is going to fail anyway.
-    if let Some(image_handle_ident) = image_handle_ident {
-        // Convert the system table arg (either `SystemTable<Boot>` or
-        // `*const c_void`) to a pointer of the correct type.
-        let system_table_ptr = if generated_args {
-            quote!(#system_table_ident.cast())
-        } else {
-            quote!(#system_table_ident.as_ptr().cast())
-        };
-
-        f.block.stmts.insert(
-            0,
-            parse_quote! {
-                unsafe {
-                    ::uefi::boot::set_image_handle(#image_handle_ident);
-                    ::uefi::table::set_system_table(#system_table_ptr);
-                }
-            },
-        );
-    }
 
+    let unsafety = &f.sig.unsafety;
     let fn_ident = &f.sig.ident;
-    // Get an iterator of the function inputs types. This is needed instead of
-    // directly using `sig.inputs` because patterns you can use in fn items like
-    // `mut <arg>` aren't valid in fn pointers.
-    let fn_inputs = f.sig.inputs.iter().map(|arg| match arg {
-        FnArg::Receiver(arg) => quote!(#arg),
-        FnArg::Typed(arg) => {
-            let ty = &arg.ty;
-            quote!(#ty)
-        }
-    });
     let fn_output = &f.sig.output;
 
     // Get the expected argument types for the main function.
-    let expected_args = if generated_args {
-        quote!(::uefi::Handle, *const core::ffi::c_void)
-    } else {
-        quote!(
-            ::uefi::Handle,
-            ::uefi::table::SystemTable<::uefi::table::Boot>
-        )
-    };
+    let expected_args = quote!(::uefi::Handle, *const core::ffi::c_void);
 
     let fn_type_check = quote_spanned! {signature_span=>
         // Cast from the function type to a function pointer with the same
@@ -281,7 +207,7 @@ pub fn entry(args: TokenStream, input: TokenStream) -> TokenStream {
             // The expected fn pointer type.
             #unsafety extern "efiapi" fn(#expected_args) -> ::uefi::Status =
             // Cast from a fn item to a function pointer.
-            #fn_ident as #unsafety extern "efiapi" fn(#(#fn_inputs),*) #fn_output;
+            #fn_ident as #unsafety extern "efiapi" fn(#expected_args) #fn_output;
     };
 
     let result = quote! {

uefi-macros/tests/ui/fail/entry_bad_abi.rs

@@ -1,11 +1,8 @@
-#![allow(unused_imports)]
 #![no_main]
-#![allow(deprecated)]
 
 use uefi::prelude::*;
-use uefi_macros::entry;
 
 #[entry]
-extern "C" fn main(_handle: Handle, _st: SystemTable<Boot>) -> Status {
+extern "C" fn main() -> Status {
     Status::SUCCESS
 }

uefi-macros/tests/ui/fail/entry_bad_abi.stderr

@@ -1,5 +1,5 @@
-error: Entry method must have no ABI modifier
- --> tests/ui/fail/entry_bad_abi.rs:9:1
+error: Entry function must have no ABI modifier
+ --> tests/ui/fail/entry_bad_abi.rs:6:1
   |
-9 | extern "C" fn main(_handle: Handle, _st: SystemTable<Boot>) -> Status {
+6 | extern "C" fn main() -> Status {
   | ^^^^^^

uefi-macros/tests/ui/fail/entry_bad_arg.rs

@@ -1,11 +1,8 @@
-#![allow(unused_imports)]
 #![no_main]
-#![allow(deprecated)]
 
 use uefi::prelude::*;
-use uefi_macros::entry;
 
 #[entry]
-fn main(_handle: Handle, _st: SystemTable<Boot>, _x: usize) -> Status {
+fn main(_x: usize) -> Status {
     Status::SUCCESS
 }

uefi-macros/tests/ui/fail/entry_bad_arg.stderr

@@ -1,8 +1,5 @@
-error[E0308]: mismatched types
- --> tests/ui/fail/entry_bad_arg.rs:9:1
+error: Entry function must have no arguments
+ --> tests/ui/fail/entry_bad_arg.rs:6:9
   |
-9 | fn main(_handle: Handle, _st: SystemTable<Boot>, _x: usize) -> Status {
-  | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ incorrect number of function parameters
-  |
-  = note: expected fn pointer `extern "efiapi" fn(uefi::Handle, uefi::prelude::SystemTable<uefi::prelude::Boot>) -> uefi::Status`
-             found fn pointer `extern "efiapi" fn(uefi::Handle, uefi::prelude::SystemTable<uefi::prelude::Boot>, usize) -> uefi::Status`
+6 | fn main(_x: usize) -> Status {
+  |         ^^

uefi-macros/tests/ui/fail/entry_bad_async.rs

@@ -1,10 +1,8 @@
-#![allow(unused_imports)]
 #![no_main]
 
 use uefi::prelude::*;
-use uefi_macros::entry;
 
 #[entry]
-async fn main(_handle: Handle, _st: SystemTable<Boot>) -> Status {
+async fn main() -> Status {
     Status::SUCCESS
 }

uefi-macros/tests/ui/fail/entry_bad_async.stderr

@@ -1,5 +1,5 @@
-error: Entry method should not be async
- --> tests/ui/fail/entry_bad_async.rs:8:1
+error: Entry function should not be async
+ --> tests/ui/fail/entry_bad_async.rs:6:1
   |
-8 | async fn main(_handle: Handle, _st: SystemTable<Boot>) -> Status {
+6 | async fn main() -> Status {
   | ^^^^^

uefi-macros/tests/ui/fail/entry_bad_attr_arg.rs

@@ -1,10 +1,8 @@
-#![allow(unused_imports)]
 #![no_main]
 
 use uefi::prelude::*;
-use uefi_macros::entry;
 
 #[entry(some_arg)]
-fn main(_handle: Handle, _st: SystemTable<Boot>) -> Status {
+fn main() -> Status {
     Status::SUCCESS
 }

uefi-macros/tests/ui/fail/entry_bad_attr_arg.stderr

@@ -1,5 +1,5 @@
 error: Entry attribute accepts no arguments
- --> tests/ui/fail/entry_bad_attr_arg.rs:7:9
+ --> tests/ui/fail/entry_bad_attr_arg.rs:5:9
   |
-7 | #[entry(some_arg)]
+5 | #[entry(some_arg)]
   |         ^^^^^^^^

uefi-macros/tests/ui/fail/entry_bad_const.rs

@@ -1,10 +1,8 @@
-#![allow(unused_imports)]
 #![no_main]
 
 use uefi::prelude::*;
-use uefi_macros::entry;
 
 #[entry]
-const fn main(_handle: Handle, _st: SystemTable<Boot>) -> Status {
+const fn main() -> Status {
     Status::SUCCESS
 }

uefi-macros/tests/ui/fail/entry_bad_const.stderr

@@ -1,5 +1,5 @@
-error: Entry method should not be const
- --> tests/ui/fail/entry_bad_const.rs:8:1
+error: Entry function should not be const
+ --> tests/ui/fail/entry_bad_const.rs:6:1
   |
-8 | const fn main(_handle: Handle, _st: SystemTable<Boot>) -> Status {
+6 | const fn main() -> Status {
   | ^^^^^

uefi-macros/tests/ui/fail/entry_bad_generic.rs

@@ -1,10 +1,8 @@
-#![allow(unused_imports)]
 #![no_main]
 
 use uefi::prelude::*;
-use uefi_macros::entry;
 
 #[entry]
-fn main<T>(_handle: Handle, _st: SystemTable<Boot>) -> Status {
+fn main<T>() -> Status {
     Status::SUCCESS
 }

uefi-macros/tests/ui/fail/entry_bad_generic.stderr

@@ -1,5 +1,5 @@
-error: Entry method should not be generic
- --> tests/ui/fail/entry_bad_generic.rs:8:9
+error: Entry function should not be generic
+ --> tests/ui/fail/entry_bad_generic.rs:6:9
   |
-8 | fn main<T>(_handle: Handle, _st: SystemTable<Boot>) -> Status {
+6 | fn main<T>() -> Status {
   |         ^

uefi-macros/tests/ui/fail/entry_bad_return_type.rs

@@ -1,11 +1,8 @@
-#![allow(unused_imports)]
 #![no_main]
-#![allow(deprecated)]
 
 use uefi::prelude::*;
-use uefi_macros::entry;
 
 #[entry]
-fn main(_handle: Handle, _st: SystemTable<Boot>) -> bool {
+fn main() -> bool {
     false
 }

uefi-macros/tests/ui/fail/entry_bad_return_type.stderr

@@ -1,8 +1,8 @@
 error[E0308]: mismatched types
- --> tests/ui/fail/entry_bad_return_type.rs:9:1
+ --> tests/ui/fail/entry_bad_return_type.rs:6:1
   |
-9 | fn main(_handle: Handle, _st: SystemTable<Boot>) -> bool {
-  | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected `Status`, found `bool`
+6 | fn main() -> bool {
+  | ^^^^^^^^^^^^^^^^^ expected `Status`, found `bool`
   |
-  = note: expected fn pointer `extern "efiapi" fn(uefi::Handle, uefi::prelude::SystemTable<uefi::prelude::Boot>) -> Status`
-             found fn pointer `extern "efiapi" fn(uefi::Handle, uefi::prelude::SystemTable<uefi::prelude::Boot>) -> bool`
+  = note: expected fn pointer `extern "efiapi" fn(Handle, *const c_void) -> Status`
+             found fn pointer `extern "efiapi" fn(Handle, *const c_void) -> bool`