zksecurity / noname

Noname: a programming language to write zkapps
https://zksecurity.github.io/noname/
181 stars 47 forks source link

should constant vars be treated differently from vars? #14

Open mimoo opened 1 year ago

mimoo commented 1 year ago

const vars are defined at the top of a module like so:

const some_name: Field = .3;

they currently don't need to be uppercase because I deemed that it was not necessary. But:

The second design decision means that:

  1. const vars share the same code as vars almost everywhere, that's practical.
  2. every ExprKind::Variable must carry a ModulePath which indicates from which module they are from, even though only const vars can be from a different module. That's a bit annoying.
  3. the code that retrieves a var in the constraint writer is a bit hairy. It looks like this:
    pub fn get_local_var(&self, fn_env: &FnEnv, var_name: &str) -> VarInfo {
        // check for consts first
        let type_checker = self.current_type_checker();
        if let Some(cst_info) = type_checker.constants.get(var_name) {
            let var = Var::new_constant(cst_info.value, cst_info.typ.span);
            return VarInfo::new(var, false, Some(TyKind::Field));
        }

        // then check for local variables
        fn_env.get_local_var(var_name)
    }
mimoo commented 1 year ago

I think the downside is not significant enough for a change now, but I could see that change happening in the future if we want to be more efficient.

An uppercase might also not be the best way to detect constants. Perhaps we should reserve the const::* namespace for defining constants. And we could have something like:

cst {
  thing = 3;
  thing2 = Thing { ... };
}

// ...
cst::thing

PS: I should add this to the book