Repository: mmcloughlin/avo (go)

Issue:
build: warn/error if Mem.Index is set but Mem.Scale is 0
An easy mistake to do is to create a `Mem{Base:x, Index:Idx}` and forget to set the Scale.

I can't see the `Scale:0` being useful, so maybe just error?

Pull request:
pass: VerifyMemOperands
Introduces a pass to verify memory operands. This is added to a new
Verify pass, that in future could do much more.

Closes #125

Patch:
diff --git a/pass/pass.go b/pass/pass.go
index 4a3abd7..3fd5452 100644
--- a/pass/pass.go
+++ b/pass/pass.go
@@ -11,6 +11,7 @@ import (
 // Compile pass compiles an avo file. Upon successful completion the avo file
 // may be printed to Go assembly.
 var Compile = Concat(
+	Verify,
 	FunctionPass(PruneJumpToFollowingLabel),
 	FunctionPass(PruneDanglingLabels),
 	FunctionPass(LabelTarget),
@@ -51,6 +52,22 @@ func (p FunctionPass) Execute(f *ir.File) error {
 	return nil
 }
 
+// InstructionPass is a convenience for implementing a full file pass with a
+// function that operates on each Instruction independently.
+type InstructionPass func(*ir.Instruction) error
+
+// Execute calls p on every instruction in the file. Exits on the first error.
+func (p InstructionPass) Execute(f *ir.File) error {
+	for _, fn := range f.Functions() {
+		for _, i := range fn.Instructions() {
+			if err := p(i); err != nil {
+				return err
+			}
+		}
+	}
+	return nil
+}
+
 // Concat returns a pass that executes the given passes in order, stopping on the first error.
 func Concat(passes ...Interface) Interface {
 	return Func(func(f *ir.File) error {
diff --git a/pass/verify.go b/pass/verify.go
new file mode 100644
index 0000000..1e7b368
--- /dev/null
+++ b/pass/verify.go
@@ -0,0 +1,32 @@
+package pass
+
+import (
+	"errors"
+
+	"github.com/mmcloughlin/avo/ir"
+	"github.com/mmcloughlin/avo/operand"
+)
+
+// Verify pass validates an avo file.
+var Verify = Concat(
+	InstructionPass(VerifyMemOperands),
+)
+
+// VerifyMemOperands checks the instruction's memory operands.
+func VerifyMemOperands(i *ir.Instruction) error {
+	for _, op := range i.Operands {
+		m, ok := op.(operand.Mem)
+		if !ok {
+			continue
+		}
+
+		if m.Base == nil {
+			return errors.New("bad memory operand: missing base register")
+		}
+
+		if m.Index != nil && m.Scale == 0 {
+			return errors.New("bad memory operand: index register with scale 0")
+		}
+	}
+	return nil
+}


Test patch:
diff --git a/pass/verify_test.go b/pass/verify_test.go
new file mode 100644
index 0000000..97d4bc2
--- /dev/null
+++ b/pass/verify_test.go
@@ -0,0 +1,58 @@
+package pass
+
+import (
+	"strings"
+	"testing"
+
+	"github.com/mmcloughlin/avo/ir"
+	"github.com/mmcloughlin/avo/operand"
+	"github.com/mmcloughlin/avo/reg"
+)
+
+func TestVerifyMemOperands(t *testing.T) {
+	i := &ir.Instruction{
+		Operands: []operand.Op{
+			reg.RAX,
+			operand.Mem{
+				Base: reg.R10,
+				Disp: 42,
+			},
+		},
+	}
+	if err := VerifyMemOperands(i); err != nil {
+		t.Fatal(err)
+	}
+}
+
+func TestVerifyMemOperandsErrors(t *testing.T) {
+	cases := []struct {
+		Operands       []operand.Op
+		ErrorSubstring string
+	}{
+		{
+			Operands: []operand.Op{
+				reg.RAX,
+				operand.Mem{
+					Disp: 42,
+				},
+			},
+			ErrorSubstring: "missing base",
+		},
+		{
+			Operands: []operand.Op{
+				operand.Mem{
+					Base:  reg.EBX,
+					Index: reg.R9L,
+				},
+				reg.ECX,
+			},
+			ErrorSubstring: "index register with scale 0",
+		},
+	}
+	for _, c := range cases {
+		i := &ir.Instruction{Operands: c.Operands}
+		if err := VerifyMemOperands(i); err == nil || !strings.Contains(err.Error(), c.ErrorSubstring) {
+			t.Errorf("got error %v; expected error to contain %q", err, c.ErrorSubstring)
+		}
+	}
+}