Make param and witness modules optional in SimplicityHL programs

[coremoon]

Project

core | compiler

Describe the feature

Problem

It seems to me an unnecessary boilerplate for programs without parameters or witnesses to require:

mod param {} 
mod witness {}
fn main() {}

Solution

Modify src/ast.rs - analyze_named_module() function:

Before:

let witness_module = iter
    .next()
    .ok_or(Error::ModuleRequired(name.shallow_clone()))
    .with_span(from)?;

After:

let witness_module = iter.next();
if iter.next().is_some() {
    return Err(Error::ModuleRedefined(name)).with_span(from);
}
let mut map = HashMap::new();
if let Some(witness_module) = witness_module {
    for assignment in witness_module.assignments() {
        if map.contains_key(assignment.name()) {
            return Err(Error::WitnessReassigned(assignment.name().shallow_clone()))
                .with_span(assignment);
        }
        map.insert(
            assignment.name().shallow_clone(),
            assignment.value().clone(),
        );
    }
}
Ok(map)

Why: Stores the result without immediate error, checks for duplicates, then optionally processes the module if it exists.

Result: Modules are now optional instead of required.

Labels: enhancement, usability

[apoelstra]

I don't think these are required. I've never put them in any of my programs.

[apoelstra]

analyze_named_module is only ever called by WitnessValues::analyze and witness::Arguments::analyze which isn't called from this crate.

I suspect this bug is actually with the web IDE, which IIRC does enforce this requirement. Though maybe the fix belongs to this repo.

[apoelstra]

The diff I would propose is

diff --git a/src/ast.rs b/src/ast.rs
index ffac991..1ccf96a 100644
--- a/src/ast.rs
+++ b/src/ast.rs
@@ -1442,10 +1442,10 @@ fn analyze_named_module(
         ModuleItem::Module(module) if module.name == name => Some(module),
         _ => None,
     });
-    let witness_module = iter
-        .next()
-        .ok_or(Error::ModuleRequired(name.shallow_clone()))
-        .with_span(from)?;
+    let witness_module = match iter.next() {
+        Some(module) => module,
+        None => return Ok(HashMap::new()), // "not present" is equivalent to empty
+    };
     if iter.next().is_some() {
         return Err(Error::ModuleRedefined(name)).with_span(from);
     }
diff --git a/src/error.rs b/src/error.rs
index d6667d9..c14ed78 100644
--- a/src/error.rs
+++ b/src/error.rs
@@ -335,7 +335,6 @@ pub enum Error {
     WitnessTypeMismatch(WitnessName, ResolvedType, ResolvedType),
     WitnessReassigned(WitnessName),
     WitnessOutsideMain,
-    ModuleRequired(ModuleName),
     ModuleRedefined(ModuleName),
     ArgumentMissing(WitnessName),
     ArgumentTypeMismatch(WitnessName, ResolvedType, ResolvedType),
@@ -465,10 +464,6 @@ impl fmt::Display for Error {
                 f,
                 "Witness expressions are not allowed outside the `main` function"
             ),
-            Error::ModuleRequired(name) => write!(
-                f,
-                "Required module `{name}` is missing"
-            ),
             Error::ModuleRedefined(name) => write!(
                 f,
                 "Module `{name}` is defined twice"`

which avoids reindenting code, so it's quite a bit shorter (except that it also correctly deletes the now-unused error variant).

[coremoon]

I don't think these are required. I've never put them in any of my programs.

I can tell that they are required whenever cargo uses simplicityhl