Merge #193

193: remove unused check r=japaric a=japaric

that was added in #140 but it's no longer required

this PR also adds some cfail tests to test the updated code

Co-authored-by: Jorge Aparicio <jorge@japaric.io>

macros/src/check.rs

@@ -1,7 +1,7 @@
 use std::{collections::HashSet, iter};
 
 use proc_macro2::Span;
-use syn::{parse, spanned::Spanned, Block, Expr, Stmt};
+use syn::parse;
 
 use crate::syntax::App;
 
@@ -35,12 +35,22 @@ pub fn app(app: &App) -> parse::Result<()> {
         }
     }
 
-    // Check that `init` returns `LateResources` if there's any declared late resource
-    if !app.init.returns_late_resources && app.resources.iter().any(|(_, res)| res.expr.is_none()) {
-        return Err(parse::Error::new(
-            app.init.span,
-            "late resources have been specified so `init` must return `init::LateResources`",
-        ));
+    if app.resources.iter().any(|(_, res)| res.expr.is_none()) {
+        // Check that `init` returns `LateResources` if there's any declared late resource
+        if !app.init.returns_late_resources {
+            return Err(parse::Error::new(
+                app.init.span,
+                "late resources have been specified so `init` must return `init::LateResources`",
+            ));
+        }
+    } else if app.init.returns_late_resources {
+        // If there are no late resources the signature should be `fn(init::Context)`
+        if app.init.returns_late_resources {
+            return Err(parse::Error::new(
+                app.init.span,
+                "`init` signature must be `fn(init::Context)` if there are no late resources",
+            ));
+        }
     }
 
     // Check that all referenced tasks have been declared
@@ -108,258 +118,5 @@ pub fn app(app: &App) -> parse::Result<()> {
         }
     }
 
-    // Check that `init` contains no early returns *if* late resources exist and `init` signature is
-    // `fn()`
-    if app.resources.values().any(|res| res.expr.is_none()) {
-        if !app.init.returns_late_resources {
-            for stmt in &app.init.stmts {
-                noreturn_stmt(stmt)?;
-            }
-        }
-    } else if app.init.returns_late_resources {
-        return Err(parse::Error::new(
-            Span::call_site(),
-            "`init` signature must be `fn(init::Context)` if there are no late resources",
-        ));
-    }
-
-    Ok(())
-}
-
-// checks that the given block contains no instance of `return`
-fn noreturn_block(block: &Block) -> Result<(), parse::Error> {
-    for stmt in &block.stmts {
-        noreturn_stmt(stmt)?;
-    }
-
-    Ok(())
-}
-
-// checks that the given statement contains no instance of `return`
-fn noreturn_stmt(stmt: &Stmt) -> Result<(), parse::Error> {
-    match stmt {
-        // `let x = ..` -- this may contain a return in the RHS
-        Stmt::Local(local) => {
-            if let Some(ref init) = local.init {
-                noreturn_expr(&init.1)?
-            }
-        }
-
-        // items have no effect on control flow
-        Stmt::Item(..) => {}
-
-        Stmt::Expr(expr) => noreturn_expr(expr)?,
-
-        Stmt::Semi(expr, ..) => noreturn_expr(expr)?,
-    }
-
-    Ok(())
-}
-
-// checks that the given expression contains no `return`
-fn noreturn_expr(expr: &Expr) -> Result<(), parse::Error> {
-    match expr {
-        Expr::Box(b) => noreturn_expr(&b.expr)?,
-
-        Expr::InPlace(ip) => {
-            noreturn_expr(&ip.place)?;
-            noreturn_expr(&ip.value)?;
-        }
-
-        Expr::Array(a) => {
-            for elem in &a.elems {
-                noreturn_expr(elem)?;
-            }
-        }
-
-        Expr::Call(c) => {
-            noreturn_expr(&c.func)?;
-
-            for arg in &c.args {
-                noreturn_expr(arg)?;
-            }
-        }
-
-        Expr::MethodCall(mc) => {
-            noreturn_expr(&mc.receiver)?;
-
-            for arg in &mc.args {
-                noreturn_expr(arg)?;
-            }
-        }
-
-        Expr::Tuple(t) => {
-            for elem in &t.elems {
-                noreturn_expr(elem)?;
-            }
-        }
-
-        Expr::Binary(b) => {
-            noreturn_expr(&b.left)?;
-            noreturn_expr(&b.right)?;
-        }
-
-        Expr::Unary(u) => {
-            noreturn_expr(&u.expr)?;
-        }
-
-        Expr::Lit(..) => {}
-
-        Expr::Cast(c) => {
-            noreturn_expr(&c.expr)?;
-        }
-
-        Expr::Type(t) => {
-            noreturn_expr(&t.expr)?;
-        }
-
-        Expr::Let(l) => {
-            noreturn_expr(&l.expr)?;
-        }
-
-        Expr::If(i) => {
-            noreturn_expr(&i.cond)?;
-
-            noreturn_block(&i.then_branch)?;
-
-            if let Some(ref e) = i.else_branch {
-                noreturn_expr(&e.1)?;
-            }
-        }
-
-        Expr::While(w) => {
-            noreturn_expr(&w.cond)?;
-            noreturn_block(&w.body)?;
-        }
-
-        Expr::ForLoop(fl) => {
-            noreturn_expr(&fl.expr)?;
-            noreturn_block(&fl.body)?;
-        }
-
-        Expr::Loop(l) => {
-            noreturn_block(&l.body)?;
-        }
-
-        Expr::Match(m) => {
-            noreturn_expr(&m.expr)?;
-
-            for arm in &m.arms {
-                if let Some(g) = &arm.guard {
-                    noreturn_expr(&g.1)?;
-                }
-
-                noreturn_expr(&arm.body)?;
-            }
-        }
-
-        // we don't care about `return`s inside closures
-        Expr::Closure(..) => {}
-
-        Expr::Unsafe(u) => {
-            noreturn_block(&u.block)?;
-        }
-
-        Expr::Block(b) => {
-            noreturn_block(&b.block)?;
-        }
-
-        Expr::Assign(a) => {
-            noreturn_expr(&a.left)?;
-            noreturn_expr(&a.right)?;
-        }
-
-        Expr::AssignOp(ao) => {
-            noreturn_expr(&ao.left)?;
-            noreturn_expr(&ao.right)?;
-        }
-
-        Expr::Field(f) => {
-            noreturn_expr(&f.base)?;
-        }
-
-        Expr::Index(i) => {
-            noreturn_expr(&i.expr)?;
-            noreturn_expr(&i.index)?;
-        }
-
-        Expr::Range(r) => {
-            if let Some(ref f) = r.from {
-                noreturn_expr(f)?;
-            }
-
-            if let Some(ref t) = r.to {
-                noreturn_expr(t)?;
-            }
-        }
-
-        Expr::Path(..) => {}
-
-        Expr::Reference(r) => {
-            noreturn_expr(&r.expr)?;
-        }
-
-        Expr::Break(b) => {
-            if let Some(ref e) = b.expr {
-                noreturn_expr(e)?;
-            }
-        }
-
-        Expr::Continue(..) => {}
-
-        Expr::Return(r) => {
-            return Err(parse::Error::new(
-                r.span(),
-                "`init` is *not* allowed to early return",
-            ));
-        }
-
-        // we can not analyze this
-        Expr::Macro(..) => {}
-
-        Expr::Struct(s) => {
-            for field in &s.fields {
-                noreturn_expr(&field.expr)?;
-            }
-
-            if let Some(ref rest) = s.rest {
-                noreturn_expr(rest)?;
-            }
-        }
-
-        Expr::Repeat(r) => {
-            noreturn_expr(&r.expr)?;
-            noreturn_expr(&r.len)?;
-        }
-
-        Expr::Paren(p) => {
-            noreturn_expr(&p.expr)?;
-        }
-
-        Expr::Group(g) => {
-            noreturn_expr(&g.expr)?;
-        }
-
-        Expr::Try(t) => {
-            noreturn_expr(&t.expr)?;
-        }
-
-        // we don't care about `return`s inside async blocks
-        Expr::Async(..) => {}
-
-        Expr::TryBlock(tb) => {
-            noreturn_block(&tb.block)?;
-        }
-
-        Expr::Yield(y) => {
-            if let Some(expr) = &y.expr {
-                noreturn_expr(expr)?;
-            }
-        }
-
-        // we can not analyze this
-        Expr::Verbatim(..) => {}
-    }
-
     Ok(())
 }

tests/cfail/init-extra-late-resources.rs

@@ -0,0 +1,15 @@
+#![no_main]
+#![no_std]
+
+extern crate lm3s6965;
+extern crate panic_halt;
+extern crate rtfm;
+
+use rtfm::app;
+
+#[app(device = lm3s6965)]
+const APP: () = {
+    #[init]
+    fn init(_: init::Context) -> init::LateResources {}
+    //~^ error: `init` signature must be `fn(init::Context)` if there are no late resources
+};

tests/cfail/init-missing-late-resources.rs

@@ -0,0 +1,17 @@
+#![no_main]
+#![no_std]
+
+extern crate lm3s6965;
+extern crate panic_halt;
+extern crate rtfm;
+
+use rtfm::app;
+
+#[app(device = lm3s6965)]
+const APP: () = {
+    static mut X: i32 = ();
+
+    #[init]
+    fn init(_: init::Context) {}
+    //~^ error: late resources have been specified so `init` must return `init::LateResources`
+};