feat: improve error reporting around functions (#494)

Improve error reporting around functions

This improves the error reporting around functions, such as arising from
defun and defpurefun.  Specifically, it highlights only the function
name when there is a general error related to the function (e.g. its
redeclared, etc).

pkg/corset/declaration.go

@@ -815,11 +815,9 @@ func (p *DefProperty) Lisp() sexp.SExp {
 // parameters).  In contrast, an impure function can access those columns
 // defined within its enclosing context.
 type DefFun struct {
-	name string
+	symbol *FunctionName
 	// Parameters
 	parameters []*DefParameter
-	//
-	binding DefunBinding
 }
 
 // IsFunction is always true for a function definition!
@@ -831,7 +829,7 @@ func (p *DefFun) IsFunction() bool {
 // which is not permitted to access any columns from the enclosing environment
 // (either directly itself, or indirectly via functions it calls).
 func (p *DefFun) IsPure() bool {
-	return p.binding.pure
+	return p.symbol.binding.pure
 }
 
 // Parameters returns information about the parameters defined by this
@@ -842,30 +840,30 @@ func (p *DefFun) Parameters() []*DefParameter {
 
 // Body Access information about the parameters defined by this declaration.
 func (p *DefFun) Body() Expr {
-	return p.binding.body
+	return p.symbol.binding.body
 }
 
 // Binding returns the allocated binding for this symbol (which may or may not
 // be finalised).
 func (p *DefFun) Binding() Binding {
-	return &p.binding
+	return p.symbol.binding
 }
 
 // Name of symbol being defined
 func (p *DefFun) Name() string {
-	return p.name
+	return p.symbol.name
 }
 
 // Definitions returns the set of symbols defined by this declaration.  Observe
 // that these may not yet have been finalised.
 func (p *DefFun) Definitions() util.Iterator[SymbolDefinition] {
-	iter := util.NewUnitIterator(p)
-	return util.NewCastIterator[*DefFun, SymbolDefinition](iter)
+	iter := util.NewUnitIterator(p.symbol)
+	return util.NewCastIterator[*FunctionName, SymbolDefinition](iter)
 }
 
 // Dependencies needed to signal declaration.
 func (p *DefFun) Dependencies() util.Iterator[Symbol] {
-	deps := p.binding.body.Dependencies()
+	deps := p.symbol.binding.body.Dependencies()
 	ndeps := make([]Symbol, 0)
 	// Filter out all parameters declared in this function, since these are not
 	// external dependencies.
@@ -881,21 +879,21 @@ func (p *DefFun) Dependencies() util.Iterator[Symbol] {
 // Defines checks whether this declaration defines the given symbol.  The symbol
 // in question needs to have been resolved already for this to make sense.
 func (p *DefFun) Defines(symbol Symbol) bool {
-	return &p.binding == symbol.Binding()
+	return p.symbol.binding == symbol.Binding()
 }
 
 // IsFinalised checks whether this declaration has already been finalised.  If
 // so, then we don't need to finalise it again.
 func (p *DefFun) IsFinalised() bool {
-	return p.binding.IsFinalised()
+	return p.symbol.binding.IsFinalised()
 }
 
 // Lisp converts this node into its lisp representation.  This is primarily used
 // for debugging purposes.
 func (p *DefFun) Lisp() sexp.SExp {
 	return sexp.NewList([]sexp.SExp{
 		sexp.NewSymbol("defun"),
-		sexp.NewSymbol(p.name),
+		sexp.NewSymbol(p.symbol.name),
 		sexp.NewSymbol("..."), // todo
 	})
 }

pkg/corset/parser.go

@@ -714,7 +714,7 @@ func (p *Parser) parseDefProperty(elements []sexp.SExp) (Declaration, []SyntaxEr
 // Parse a permutation declaration
 func (p *Parser) parseDefFun(pure bool, elements []sexp.SExp) (Declaration, []SyntaxError) {
 	var (
-		name      string
+		name      *sexp.Symbol
 		ret       Type
 		params    []*DefParameter
 		errors    []SyntaxError
@@ -741,11 +741,14 @@ func (p *Parser) parseDefFun(pure bool, elements []sexp.SExp) (Declaration, []Sy
 	}
 	// Construct binding
 	binding := NewDefunBinding(pure, paramTypes, ret, body)
+	fn_name := NewFunctionName(name.Value, &binding)
+	// Update source mapping
+	p.mapSourceNode(name, fn_name)
 	//
-	return &DefFun{name, params, binding}, nil
+	return &DefFun{fn_name, params}, nil
 }
 
-func (p *Parser) parseFunctionSignature(elements []sexp.SExp) (string, Type, []*DefParameter, []SyntaxError) {
+func (p *Parser) parseFunctionSignature(elements []sexp.SExp) (*sexp.Symbol, Type, []*DefParameter, []SyntaxError) {
 	var (
 		params []*DefParameter = make([]*DefParameter, len(elements)-1)
 	)
@@ -761,13 +764,13 @@ func (p *Parser) parseFunctionSignature(elements []sexp.SExp) (string, Type, []*
 	}
 	// Check for any errors arising
 	if len(errors) > 0 {
-		return "", nil, nil, errors
+		return nil, nil, nil, errors
 	}
 	//
 	return name, ret, params, nil
 }
 
-func (p *Parser) parseFunctionNameReturn(element sexp.SExp) (string, Type, bool, []SyntaxError) {
+func (p *Parser) parseFunctionNameReturn(element sexp.SExp) (*sexp.Symbol, Type, bool, []SyntaxError) {
 	var (
 		err    *SyntaxError
 		name   sexp.SExp
@@ -786,7 +789,7 @@ func (p *Parser) parseFunctionNameReturn(element sexp.SExp) (string, Type, bool,
 			// Check what we have
 			if symbol == nil {
 				err := p.translator.SyntaxError(element, "modifier expected")
-				return "", nil, false, []SyntaxError{*err}
+				return nil, nil, false, []SyntaxError{*err}
 			} else if i == 0 {
 				name = symbol
 			} else {
@@ -795,19 +798,19 @@ func (p *Parser) parseFunctionNameReturn(element sexp.SExp) (string, Type, bool,
 					forced = true
 				default:
 					if ret, _, err = p.parseType(element); err != nil {
-						return "", nil, false, []SyntaxError{*err}
+						return nil, nil, false, []SyntaxError{*err}
 					}
 				}
 			}
 		}
 	}
 	//
 	if isFunIdentifier(name) {
-		return name.AsSymbol().Value, ret, forced, nil
+		return name.AsSymbol(), ret, forced, nil
 	} else {
 		// Must be non-identifier symbol
 		err = p.translator.SyntaxError(element, "invalid function name")
-		return "", nil, false, []SyntaxError{*err}
+		return nil, nil, false, []SyntaxError{*err}
 	}
 }
 

pkg/corset/resolver.go

@@ -455,7 +455,7 @@ func (r *resolver) finaliseDefFunInModule(enclosing Scope, decl *DefFun) []Synta
 	datatype, errors := r.finaliseExpressionInModule(scope, decl.Body())
 	// Finalise declaration
 	if len(errors) == 0 {
-		decl.binding.Finalise(datatype)
+		decl.symbol.binding.Finalise(datatype)
 	}
 	// Done
 	return errors

pkg/corset/symbol.go

@@ -65,6 +65,15 @@ func NewColumnName(name string) *ColumnName {
 	return &ColumnName{name, false, nil, false}
 }
 
+// FunctionName represents a name used in a position where it can only be
+// resolved as a function.
+type FunctionName = Name[*DefunBinding]
+
+// NewFunctionName construct a new column name which is (initially) unresolved.
+func NewFunctionName(name string, binding *DefunBinding) *FunctionName {
+	return &FunctionName{name, true, binding, true}
+}
+
 // Name represents a name within some syntactic item.  Essentially this wraps a
 // string and provides a mechanism for it to be associated with source line
 // information.

testdata/purefun_invalid_10.lisp

@@ -1,3 +1,3 @@
-;;error:3:1-29:symbol * already declared
+;;error:3:14-15:symbol * already declared
 ;; Attempt to overload intrinsic.
 (defpurefun (* x y) (* x y))

testdata/purefun_invalid_11.lisp

@@ -1,4 +1,4 @@
-;;error:4:1-50:symbol eq already declared
+;;error:4:14-16:symbol eq already declared
 ;; Duplicate overload is always a syntax error.
 (defpurefun (eq (x :binary) (y :binary)) (- x y))
 (defpurefun (eq (x :binary) (y :binary)) (+ x y))

testdata/purefun_invalid_12.lisp

@@ -1,4 +1,4 @@
-;;error:4:1-25:symbol eq already declared
+;;error:4:9-11:symbol eq already declared
 ;; Cannot overload pure with impure, and vice versa.
 (defpurefun (eq (x :binary) (y :binary)) (- x y))
 (defun (eq x y) (+ x y))