Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace builtin map by ordered map for deterministic iteration #2

Merged
merged 11 commits into from
Jul 18, 2024
4 changes: 3 additions & 1 deletion .github/workflows/go.yml
Original file line number Diff line number Diff line change
Expand Up @@ -25,4 +25,6 @@ jobs:
run: go test -v ./... -race -coverprofile=coverage.txt -covermode=atomic

- name: Codecov
uses: codecov/codecov-action@v2.1.0
uses: codecov/codecov-action@v4
with:
token: ${{ secrets.CODECOV_TOKEN }}
4 changes: 4 additions & 0 deletions codecov.yml
Original file line number Diff line number Diff line change
@@ -1,2 +1,6 @@
comment:
layout: "reach, diff, flags, files"
behavior: default

ignore:
- "**/main.go"
2 changes: 1 addition & 1 deletion engine/atom.go
Original file line number Diff line number Diff line change
Expand Up @@ -234,7 +234,7 @@ func NewAtom(name string) Atom {
// WriteTerm outputs the Atom to an io.Writer.
func (a Atom) WriteTerm(w io.Writer, opts *WriteOptions, _ *Env) error {
ew := errWriter{w: w}
openClose := (opts.left != (operator{}) || opts.right != (operator{})) && opts.ops.defined(a)
openClose := (opts.left != (operator{}) || opts.right != (operator{})) && opts.getOps().defined(a)

if openClose {
if opts.left.name != 0 && opts.left.specifier.class() == operatorClassPrefix {
Expand Down
5 changes: 3 additions & 2 deletions engine/atom_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package engine
import (
"bytes"
"github.com/stretchr/testify/assert"
orderedmap "github.com/wk8/go-ordered-map/v2"
"testing"
)

Expand All @@ -23,8 +24,8 @@ func TestAtom_WriteTerm(t *testing.T) {
{name: `{}`, opts: WriteOptions{quoted: false}, output: `{}`},
{name: `{}`, opts: WriteOptions{quoted: true}, output: `{}`},
{name: `-`, output: `-`},
{name: `-`, opts: WriteOptions{ops: operators{atomPlus: {}, atomMinus: {}}, left: operator{specifier: operatorSpecifierFY, name: atomPlus}}, output: ` (-)`},
{name: `-`, opts: WriteOptions{ops: operators{atomPlus: {}, atomMinus: {}}, right: operator{name: atomPlus}}, output: `(-)`},
{name: `-`, opts: WriteOptions{_ops: &operators{OrderedMap: orderedmap.New[Atom, [_operatorClassLen]operator](orderedmap.WithInitialData(orderedmap.Pair[Atom, [_operatorClassLen]operator]{Key: atomPlus, Value: [3]operator{}}, orderedmap.Pair[Atom, [_operatorClassLen]operator]{Key: atomMinus, Value: [3]operator{}}))}, left: operator{specifier: operatorSpecifierFY, name: atomPlus}}, output: ` (-)`},
{name: `-`, opts: WriteOptions{_ops: &operators{OrderedMap: orderedmap.New[Atom, [_operatorClassLen]operator](orderedmap.WithInitialData(orderedmap.Pair[Atom, [_operatorClassLen]operator]{Key: atomPlus, Value: [3]operator{}}, orderedmap.Pair[Atom, [_operatorClassLen]operator]{Key: atomMinus, Value: [3]operator{}}))}, right: operator{name: atomPlus}}, output: `(-)`},
{name: `X`, opts: WriteOptions{quoted: true, left: operator{name: NewAtom(`F`)}}, output: ` 'X'`}, // So that it won't be 'F''X'.
{name: `X`, opts: WriteOptions{quoted: true, right: operator{name: NewAtom(`F`)}}, output: `'X' `}, // So that it won't be 'X''F'.
{name: `foo`, opts: WriteOptions{left: operator{name: NewAtom(`bar`)}}, output: ` foo`}, // So that it won't be barfoo.
Expand Down
48 changes: 25 additions & 23 deletions engine/builtin.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"bytes"
"context"
"errors"
orderedmap "github.com/wk8/go-ordered-map/v2"
"io"
"io/fs"
"os"
Expand Down Expand Up @@ -560,11 +561,11 @@ func Op(vm *VM, priority, specifier, op Term, k Cont, env *Env) *Promise {
}

for _, name := range names {
if class := spec.class(); vm.operators.definedInClass(name, spec.class()) {
vm.operators.remove(name, class)
if class := spec.class(); vm.getOperators().definedInClass(name, spec.class()) {
vm.getOperators().remove(name, class)
}

vm.operators.define(p, spec, name)
vm.getOperators().define(p, spec, name)
}

return k(env)
Expand All @@ -573,13 +574,13 @@ func Op(vm *VM, priority, specifier, op Term, k Cont, env *Env) *Promise {
func validateOp(vm *VM, p Integer, spec operatorSpecifier, name Atom, env *Env) *Promise {
switch name {
case atomComma:
if vm.operators.definedInClass(name, operatorClassInfix) {
if vm.getOperators().definedInClass(name, operatorClassInfix) {
return Error(permissionError(operationModify, permissionTypeOperator, name, env))
}
case atomBar:
if spec.class() != operatorClassInfix || (p > 0 && p < 1001) {
op := operationCreate
if vm.operators.definedInClass(name, operatorClassInfix) {
if vm.getOperators().definedInClass(name, operatorClassInfix) {
op = operationModify
}
return Error(permissionError(op, permissionTypeOperator, name, env))
Expand All @@ -591,11 +592,11 @@ func validateOp(vm *VM, p Integer, spec operatorSpecifier, name Atom, env *Env)
// 6.3.4.3 There shall not be an infix and a postfix Operator with the same name.
switch spec.class() {
case operatorClassInfix:
if vm.operators.definedInClass(name, operatorClassPostfix) {
if vm.getOperators().definedInClass(name, operatorClassPostfix) {
return Error(permissionError(operationCreate, permissionTypeOperator, name, env))
}
case operatorClassPostfix:
if vm.operators.definedInClass(name, operatorClassInfix) {
if vm.getOperators().definedInClass(name, operatorClassInfix) {
return Error(permissionError(operationCreate, permissionTypeOperator, name, env))
}
}
Expand Down Expand Up @@ -652,9 +653,9 @@ func CurrentOp(vm *VM, priority, specifier, op Term, k Cont, env *Env) *Promise
}

pattern := tuple(priority, specifier, op)
ks := make([]func(context.Context) *Promise, 0, len(vm.operators)*int(_operatorClassLen))
for _, ops := range vm.operators {
for _, op := range ops {
ks := make([]func(context.Context) *Promise, 0, vm.getOperators().Len()*int(_operatorClassLen))
for ops := vm.getOperators().Oldest(); ops != nil; ops = ops.Next() {
for _, op := range ops.Value {
op := op
if op == (operator{}) {
continue
Expand Down Expand Up @@ -701,12 +702,12 @@ func assertMerge(vm *VM, t Term, merge func([]clause, []clause) []clause, env *E
}

if vm.procedures == nil {
vm.procedures = map[procedureIndicator]procedure{}
vm.procedures = orderedmap.New[procedureIndicator, procedure]()
}
p, ok := vm.procedures[pi]
p, ok := vm.getProcedure(pi)
if !ok {
p = &userDefined{public: true, dynamic: true}
vm.procedures[pi] = p
vm.setProcedure(pi, p)
}

added, err := compile(t, env)
Expand Down Expand Up @@ -1066,11 +1067,11 @@ func CurrentPredicate(vm *VM, pi Term, k Cont, env *Env) *Promise {
return Error(typeError(validTypePredicateIndicator, pi, env))
}

ks := make([]func(context.Context) *Promise, 0, len(vm.procedures))
for key, p := range vm.procedures {
switch p.(type) {
ks := make([]func(context.Context) *Promise, 0, vm.procedures.Len())
for element := vm.procedures.Oldest(); element != nil; element = element.Next() {
switch element.Value.(type) {
case *userDefined:
c := key.Term()
c := element.Key.Term()
ks = append(ks, func(context.Context) *Promise {
return Unify(vm, pi, c, k, env)
})
Expand All @@ -1091,7 +1092,7 @@ func Retract(vm *VM, t Term, k Cont, env *Env) *Promise {
return Error(err)
}

p, ok := vm.procedures[pi]
p, ok := vm.getProcedure(pi)
if !ok {
return Bool(false)
}
Expand Down Expand Up @@ -1142,10 +1143,11 @@ func Abolish(vm *VM, pi Term, k Cont, env *Env) *Promise {
return Error(domainError(validDomainNotLessThanZero, arity, env))
}
key := procedureIndicator{name: name, arity: arity}
if u, ok := vm.procedures[key].(*userDefined); !ok || !u.dynamic {
p, _ := vm.getProcedure(key)
if u, ok := p.(*userDefined); !ok || !u.dynamic {
return Error(permissionError(operationModify, permissionTypeStaticProcedure, key.Term(), env))
}
delete(vm.procedures, key)
vm.procedures.Delete(key)
return k(env)
default:
return Error(typeError(validTypeInteger, arity, env))
Expand Down Expand Up @@ -1461,7 +1463,7 @@ func WriteTerm(vm *VM, streamOrAlias, t, options Term, k Cont, env *Env) *Promis
}

opts := WriteOptions{
ops: vm.operators,
_ops: vm.getOperators(),
priority: 1200,
}
iter := ListIterator{List: options, Env: env}
Expand Down Expand Up @@ -1982,7 +1984,7 @@ func Clause(vm *VM, head, body Term, k Cont, env *Env) *Promise {
return Error(typeError(validTypeCallable, body, env))
}

p, ok := vm.procedures[pi]
p, ok := vm.getProcedure(pi)
if !ok {
return Bool(false)
}
Expand Down Expand Up @@ -2749,7 +2751,7 @@ func ExpandTerm(vm *VM, term1, term2 Term, k Cont, env *Env) *Promise {
}

func expand(vm *VM, term Term, env *Env) (Term, error) {
if _, ok := vm.procedures[procedureIndicator{name: atomTermExpansion, arity: 2}]; ok {
if _, ok := vm.getProcedure(procedureIndicator{name: atomTermExpansion, arity: 2}); ok {
var ret Term
v := NewVariable()
ok, err := Call(vm, atomTermExpansion.Apply(term, v), func(env *Env) *Promise {
Expand Down
Loading
Loading