Merge pull request #226 from Consensys/225-computed-column-already-ex…

…ists

Remove `Trace.HasColumn(string)`

pkg/air/gadgets/column_sort.go

@@ -4,6 +4,7 @@ import (
 	"fmt"
 
 	"github.com/consensys/go-corset/pkg/air"
+	sc "github.com/consensys/go-corset/pkg/schema"
 	"github.com/consensys/go-corset/pkg/schema/assignment"
 )
 
@@ -36,9 +37,13 @@ func ApplyColumnSortGadget(col uint, sign bool, bitwidth uint, schema *air.Schem
 		Xdiff = Xkm1.Sub(Xk)
 		deltaName = fmt.Sprintf("-%s", name)
 	}
-	// Add delta assignment
-	deltaIndex := schema.AddAssignment(
-		assignment.NewComputedColumn(column.Context(), deltaName, Xdiff))
+	// Look up column
+	deltaIndex, ok := sc.ColumnIndexOf(schema, column.Context().Module(), deltaName)
+	// Add new column (if it does not already exist)
+	if !ok {
+		deltaIndex = schema.AddAssignment(
+			assignment.NewComputedColumn(column.Context(), deltaName, Xdiff))
+	}
 	// Add necessary bitwidth constraints
 	ApplyBitwidthGadget(deltaIndex, bitwidth, schema)
 	// Configure constraint: Delta[k] = X[k] - X[k-1]

pkg/air/gadgets/expand.go

@@ -15,6 +15,9 @@ import (
 // in the case the given expression is a direct column access by simply
 // returning the accessed column index.
 func Expand(ctx trace.Context, e air.Expr, schema *air.Schema) uint {
+	if ctx.IsVoid() || ctx.IsConflicted() {
+		panic("conflicting (or void) context")
+	}
 	//
 	if ca, ok := e.(*air.ColumnAccess); ok && ca.Shift == 0 {
 		// Optimisation possible

pkg/air/gadgets/normalisation.go

@@ -30,6 +30,10 @@ func Normalise(e air.Expr, schema *air.Schema) air.Expr {
 func ApplyPseudoInverseGadget(e air.Expr, schema *air.Schema) air.Expr {
 	// Determine enclosing module.
 	ctx := e.Context(schema)
+	// Sanity check
+	if ctx.IsVoid() || ctx.IsConflicted() {
+		panic("conflicting (or void) context")
+	}
 	// Construct inverse computation
 	ie := &Inverse{Expr: e}
 	// Determine computed column name

pkg/schema/assignment/computed_column.go

@@ -26,8 +26,8 @@ type ComputedColumn[E sc.Evaluable] struct {
 // determining expression.  More specifically, that expression is used to
 // compute the values for this column during trace expansion.
 func NewComputedColumn[E sc.Evaluable](context trace.Context, name string, expr E) *ComputedColumn[E] {
-	// FIXME: Determine computed columns type?
 	column := sc.NewColumn(context, name, &sc.FieldType{})
+	// FIXME: Determine computed columns type?
 	return &ComputedColumn[E]{column, expr}
 }
 
@@ -78,8 +78,9 @@ func (p *ComputedColumn[E]) RequiredSpillage() uint {
 func (p *ComputedColumn[E]) ExpandTrace(tr trace.Trace) error {
 	columns := tr.Columns()
 	// Check whether a column already exists with the given name.
-	if columns.HasColumn(p.Name()) {
-		return fmt.Errorf("column already exists ({%s})", p.Name())
+	if _, ok := columns.IndexOf(p.target.Context().Module(), p.Name()); ok {
+		mod := tr.Modules().Get(p.target.Context().Module())
+		return fmt.Errorf("computed column already exists ({%s.%s})", mod.Name(), p.Name())
 	}
 	// Extract length multipiler
 	multiplier := p.target.Context().LengthMultiplier()

pkg/schema/assignment/interleave.go

@@ -77,8 +77,8 @@ func (p *Interleaving) ExpandTrace(tr tr.Trace) error {
 	for i := p.Columns(); i.HasNext(); {
 		name := i.Next().Name()
 		// Sanity check no column already exists with this name.
-		if columns.HasColumn(name) {
-			return fmt.Errorf("column already exists ({%s})", name)
+		if _, ok := columns.IndexOf(ctx.Module(), name); ok {
+			return fmt.Errorf("interleaved column already exists ({%s})", name)
 		}
 	}
 	// Determine interleaving width

pkg/schema/assignment/sorted_permutation.go

@@ -127,8 +127,8 @@ func (p *SortedPermutation) ExpandTrace(tr tr.Trace) error {
 	for i := p.Columns(); i.HasNext(); {
 		name := i.Next().Name()
 		// Sanity check no column already exists with this name.
-		if columns.HasColumn(name) {
-			return fmt.Errorf("column already exists ({%s})", name)
+		if _, ok := columns.IndexOf(p.context.Module(), name); ok {
+			return fmt.Errorf("permutation column already exists ({%s})", name)
 		}
 	}
 

pkg/trace/array_trace.go

@@ -124,17 +124,6 @@ func (p arrayTraceColumnSet) Get(index uint) Column {
 	return p.trace.columns[index]
 }
 
-// HasColumn checks whether a given column exists in this column set (or not).
-func (p arrayTraceColumnSet) HasColumn(name string) bool {
-	for _, c := range p.trace.columns {
-		if c.Name() == name {
-			return true
-		}
-	}
-	// Not found
-	return false
-}
-
 // IndexOf returns the column index of the column with the given name in
 // this trace, or returns false if no such column exists.
 func (p arrayTraceColumnSet) IndexOf(module uint, name string) (uint, bool) {

pkg/trace/trace.go

@@ -23,8 +23,6 @@ type ColumnSet interface {
 	Add(column Column) uint
 	// Get the ith module in this set.
 	Get(uint) Column
-	// Check whether or not a column with the given name already exists.
-	HasColumn(string) bool
 	// Determine index of given column, or return false if this fails.
 	IndexOf(module uint, column string) (uint, bool)
 	// Returns the number of items in this array.