Skip to content

Commit

Permalink
fix: improve errors for substituted for variables (#495)
Browse files Browse the repository at this point in the history
Improve errors for substituted for variables

This improves the error reporting for substituted index variables.
For example, when the index variable is actually out-of-range then its
nice to report a sensible error about this.
  • Loading branch information
DavePearce authored Dec 22, 2024
1 parent 3aed5d4 commit 2bae347
Show file tree
Hide file tree
Showing 4 changed files with 59 additions and 2 deletions.
45 changes: 44 additions & 1 deletion pkg/corset/expression.go
Original file line number Diff line number Diff line change
Expand Up @@ -989,7 +989,9 @@ func Substitute(expr Expr, mapping map[uint]Expr, srcmap *sexp.SourceMaps[Node])
} else if e2, ok2 := mapping[b.index]; !ok2 {
return e
} else {
return e2
// Shallow copy the node to ensure it is unique and, hence, can have
// the source mapping associated with e.
nexpr = ShallowCopy(e2)
}
default:
panic(fmt.Sprintf("unknown expression (%s)", reflect.TypeOf(expr)))
Expand Down Expand Up @@ -1025,6 +1027,47 @@ func SubstituteOptional(expr Expr, mapping map[uint]Expr, srcmap *sexp.SourceMap
return expr
}

// ShallowCopy creates a copy of the expression itself, but not those
// expressions it contains (if any). This is useful in e.g. situations where we
// want to assocate different souce file information with a specific expression.
func ShallowCopy(expr Expr) Expr {
//
switch e := expr.(type) {
case *ArrayAccess:
return &ArrayAccess{e.name, e.arg, e.binding}
case *Add:
return &Add{e.Args}
case *Constant:
return &Constant{e.Val}
case *Debug:
return &Debug{e.Arg}
case *Exp:
return &Exp{e.Arg, e.Pow}
case *For:
return &For{e.Binding, e.Start, e.End, e.Body}
case *If:
return &If{e.kind, e.Condition, e.TrueBranch, e.FalseBranch}
case *Invoke:
return &Invoke{e.fn, e.signature, e.args}
case *List:
return &List{e.Args}
case *Mul:
return &Mul{e.Args}
case *Normalise:
return &Normalise{e.Arg}
case *Reduce:
return &Reduce{e.fn, e.signature, e.arg}
case *Sub:
return &Sub{e.Args}
case *Shift:
return &Shift{e.Arg, e.Shift}
case *VariableAccess:
return &VariableAccess{e.module, e.name, e.fn, e.binding}
default:
panic(fmt.Sprintf("unknown expression (%s)", reflect.TypeOf(expr)))
}
}

// DependenciesOfExpressions determines the dependencies for a given set of zero
// or more expressions.
func DependenciesOfExpressions(exprs []Expr) []Symbol {
Expand Down
2 changes: 1 addition & 1 deletion pkg/sexp/translator.go
Original file line number Diff line number Diff line change
Expand Up @@ -336,7 +336,7 @@ func translateSExpArray[T comparable](p *Translator[T], l *Array) (T, []SyntaxEr
// Check whether we found one.
if t != nil {
node, errors = (t)(l)
} else if p.list_default != nil {
} else if p.array_default != nil {
node, err := (p.array_default)(l)
// Update source mapping
if err == nil {
Expand Down
4 changes: 4 additions & 0 deletions pkg/test/invalid_corset_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -576,6 +576,10 @@ func Test_Invalid_Array_05(t *testing.T) {
CheckInvalid(t, "array_invalid_05")
}

func Test_Invalid_Array_06(t *testing.T) {
CheckInvalid(t, "array_invalid_06")
}

// ===================================================================
// Reduce
// ===================================================================
Expand Down
10 changes: 10 additions & 0 deletions testdata/array_invalid_06.lisp
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
;;error:10:36-37:array index out-of-bounds
;;error:10:31-38:void expression not permitted here
(defcolumns
(BIT :binary@prove :array [4])
(ARG :i16@loob))

(defconstraint bits ()
(- ARG
(reduce +
(for i [0:3] (* (^ 2 i) [BIT i])))))

0 comments on commit 2bae347

Please sign in to comment.