From e65dd0896c635139701c35bc52d3c28a20e2950d Mon Sep 17 00:00:00 2001 From: Benjamin DENEUX Date: Fri, 19 Jul 2024 10:16:20 +0200 Subject: [PATCH 1/3] refactor: remove global atoms table --- engine/atom.go | 45 +++++++++++++++------------------------------ engine/builtin.go | 16 ++++++++-------- engine/compound.go | 6 +++--- engine/parser.go | 28 ++++++++++++++-------------- engine/stream.go | 4 ++-- engine/term_test.go | 4 ++-- engine/vm.go | 1 + 7 files changed, 45 insertions(+), 59 deletions(-) diff --git a/engine/atom.go b/engine/atom.go index 75474f1..dfd3273 100644 --- a/engine/atom.go +++ b/engine/atom.go @@ -1,11 +1,12 @@ package engine import ( + "crypto/sha256" + "encoding/binary" "fmt" "io" "regexp" "strings" - "sync" "unicode/utf8" ) @@ -13,16 +14,6 @@ var ( quotedAtomEscapePattern = regexp.MustCompile(`[[:cntrl:]]|\\|'`) ) -var ( - atomTable = struct { - sync.RWMutex - names []string - atoms map[string]Atom - }{ - atoms: map[string]Atom{}, - } -) - // Well-known atoms. var ( atomEmpty = NewAtom("") @@ -208,27 +199,26 @@ var ( ) // Atom is a prolog atom. -type Atom uint64 +type Atom struct { + value uint64 + name string +} // NewAtom interns the given string and returns an Atom. func NewAtom(name string) Atom { // A one-char atom is just a rune. if r, n := utf8.DecodeLastRuneInString(name); r != utf8.RuneError && n == len(name) { - return Atom(r) + return Atom{uint64(r), name} } - atomTable.Lock() - defer atomTable.Unlock() + hashBytes := sha256.Sum256([]byte(name)) + hashUint64 := binary.BigEndian.Uint64(hashBytes[:8]) - a, ok := atomTable.atoms[name] - if ok { - return a - } + return Atom{hashUint64, name} +} - a = Atom(len(atomTable.names) + (utf8.MaxRune + 1)) - atomTable.atoms[name] = a - atomTable.names = append(atomTable.names, name) - return a +func NewAtomRune(v rune) Atom { + return Atom{uint64(v), string(v)} } // WriteTerm outputs the Atom to an io.Writer. @@ -237,7 +227,7 @@ func (a Atom) WriteTerm(w io.Writer, opts *WriteOptions, _ *Env) error { openClose := (opts.left != (operator{}) || opts.right != (operator{})) && opts.getOps().defined(a) if openClose { - if opts.left.name != 0 && opts.left.specifier.class() == operatorClassPrefix { + if opts.left.name.value != 0 && opts.left.specifier.class() == operatorClassPrefix { _, _ = ew.Write([]byte(" ")) } _, _ = ew.Write([]byte("(")) @@ -289,12 +279,7 @@ func (a Atom) Compare(t Term, env *Env) int { } func (a Atom) String() string { - if a <= utf8.MaxRune { - return string(rune(a)) - } - atomTable.RLock() - defer atomTable.RUnlock() - return atomTable.names[a-(utf8.MaxRune+1)] + return a.name } // Apply returns a Compound which Functor is the Atom and args are the arguments. If the arguments are empty, diff --git a/engine/builtin.go b/engine/builtin.go index 4d00e40..b3ccd46 100644 --- a/engine/builtin.go +++ b/engine/builtin.go @@ -1621,7 +1621,7 @@ func CharCode(vm *VM, char, code Term, k Cont, env *Env) *Promise { return Error(representationError(flagCharacterCode, env)) } - return Unify(vm, ch, Atom(r), k, env) + return Unify(vm, ch, NewAtomRune(r), k, env) default: return Error(typeError(validTypeInteger, code, env)) } @@ -1685,11 +1685,11 @@ func PutChar(vm *VM, streamOrAlias, char Term, k Cont, env *Env) *Promise { case Variable: return Error(InstantiationError(env)) case Atom: - if c > utf8.MaxRune { + if c.value > utf8.MaxRune { return Error(typeError(validTypeCharacter, c, env)) } - r := rune(c) + r := rune(c.value) switch _, err := s.WriteRune(r); { case errors.Is(err, errWrongIOMode): @@ -1861,7 +1861,7 @@ func GetChar(vm *VM, streamOrAlias, char Term, k Cont, env *Env) *Promise { return Error(representationError(flagCharacter, env)) } - return Unify(vm, char, Atom(r), k, env) + return Unify(vm, char, NewAtomRune(r), k, env) case io.EOF: return Unify(vm, char, atomEndOfFile, k, env) case errWrongIOMode: @@ -1941,7 +1941,7 @@ func PeekChar(vm *VM, streamOrAlias, char Term, k Cont, env *Env) *Promise { return Error(representationError(flagCharacter, env)) } - return Unify(vm, char, Atom(r), k, env) + return Unify(vm, char, NewAtomRune(r), k, env) case io.EOF: return Unify(vm, char, atomEndOfFile, k, env) case errWrongIOMode: @@ -2335,7 +2335,7 @@ func numberCharsWrite(vm *VM, num, chars Term, k Cont, env *Env) *Promise { cs := make([]Term, len(rs)) for i, r := range rs { - cs[i] = Atom(r) + cs[i] = NewAtomRune(r) } return Unify(vm, chars, List(cs...), k, env) } @@ -2584,7 +2584,7 @@ func CurrentCharConversion(vm *VM, inChar, outChar Term, k Cont, env *Env) *Prom if c1, ok := env.Resolve(inChar).(Atom); ok { r := []rune(c1.String()) if r, ok := vm.charConversions[r[0]]; ok { - return Unify(vm, outChar, Atom(r), k, env) + return Unify(vm, outChar, NewAtomRune(r), k, env) } return Unify(vm, outChar, c1, k, env) } @@ -2599,7 +2599,7 @@ func CurrentCharConversion(vm *VM, inChar, outChar Term, k Cont, env *Env) *Prom } ks[i] = func(context.Context) *Promise { - return Unify(vm, pattern, tuple(Atom(r), Atom(cr)), k, env) + return Unify(vm, pattern, tuple(NewAtomRune(r), NewAtomRune(cr)), k, env) } } return Delay(ks...) diff --git a/engine/compound.go b/engine/compound.go index 1cc1684..9a66d3d 100644 --- a/engine/compound.go +++ b/engine/compound.go @@ -204,7 +204,7 @@ func writeCompoundOpInfix(w io.Writer, c Compound, opts *WriteOptions, env *Env, (opts.right != operator{} && r >= opts.right.priority) if openClose { - if opts.left.name != 0 && opts.left.specifier.class() == operatorClassPrefix { + if opts.left.name.value != 0 && opts.left.specifier.class() == operatorClassPrefix { _, _ = fmt.Fprint(&ew, " ") } _, _ = fmt.Fprint(&ew, "(") @@ -494,7 +494,7 @@ func pair(k, v Term) Term { } func tuple(args ...Term) Term { - return Atom(0).Apply(args...) + return Atom{}.Apply(args...) } type charList string @@ -524,7 +524,7 @@ func (c charList) Arg(n int) Term { var t Term switch n { case 0: - t = Atom(r) + t = NewAtomRune(r) case 1: if i == len(c) { t = atomEmptyList diff --git a/engine/parser.go b/engine/parser.go index 2f469aa..c668991 100644 --- a/engine/parser.go +++ b/engine/parser.go @@ -463,13 +463,13 @@ func (p *Parser) op(maxPriority Integer) (Atom, error) { if p.current().kind == tokenCloseList { p.backup() } - return 0, errNoOp + return Atom{}, errNoOp case atomEmptyBlock: p.backup() if p.current().kind == tokenCloseCurly { p.backup() } - return 0, errNoOp + return Atom{}, errNoOp default: return a, nil } @@ -477,7 +477,7 @@ func (p *Parser) op(maxPriority Integer) (Atom, error) { t, err := p.next() if err != nil { - return 0, err + return Atom{}, err } switch t.kind { case tokenComma: @@ -489,7 +489,7 @@ func (p *Parser) op(maxPriority Integer) (Atom, error) { } p.backup() - return 0, errExpectation + return Atom{}, errExpectation } func (p *Parser) term0(maxPriority Integer) (Term, error) { @@ -571,7 +571,7 @@ func (p *Parser) term0Atom(maxPriority Integer) (Term, error) { return nil, errExpectation } - if p.placeholder != 0 && t == p.placeholder { + if p.placeholder.value != 0 && t == p.placeholder { if len(p.args) == 0 { return nil, errPlaceholder } @@ -616,13 +616,13 @@ func (p *Parser) atom() (Atom, error) { t, err := p.next() if err != nil { - return 0, err + return Atom{}, err } switch t.kind { case tokenOpenList: t, err := p.next() if err != nil { - return 0, err + return Atom{}, err } switch t.kind { case tokenCloseList: @@ -630,12 +630,12 @@ func (p *Parser) atom() (Atom, error) { default: p.backup() p.backup() - return 0, errExpectation + return Atom{}, errExpectation } case tokenOpenCurly: t, err := p.next() if err != nil { - return 0, err + return Atom{}, err } switch t.kind { case tokenCloseCurly: @@ -643,7 +643,7 @@ func (p *Parser) atom() (Atom, error) { default: p.backup() p.backup() - return 0, errExpectation + return Atom{}, errExpectation } case tokenDoubleQuotedList: switch p.doubleQuotes { @@ -651,18 +651,18 @@ func (p *Parser) atom() (Atom, error) { return NewAtom(unDoubleQuote(t.val)), nil default: p.backup() - return 0, errExpectation + return Atom{}, errExpectation } default: p.backup() - return 0, errExpectation + return Atom{}, errExpectation } } func (p *Parser) name() (Atom, error) { t, err := p.next() if err != nil { - return 0, err + return Atom{}, err } switch t.kind { case tokenLetterDigit, tokenGraphic, tokenSemicolon, tokenCut: @@ -671,7 +671,7 @@ func (p *Parser) name() (Atom, error) { return NewAtom(unquote(t.val)), nil default: p.backup() - return 0, errExpectation + return Atom{}, errExpectation } } diff --git a/engine/stream.go b/engine/stream.go index fe7f7da..e49b4f6 100644 --- a/engine/stream.go +++ b/engine/stream.go @@ -359,7 +359,7 @@ func (s *Stream) properties() []Term { ps = append(ps, atomOutput) } - if s.alias != 0 { + if s.alias.value != 0 { ps = append(ps, atomAlias.Apply(s.alias)) } @@ -510,7 +510,7 @@ type streams struct { } func (ss *streams) add(s *Stream) { - if s.alias != 0 { + if s.alias.value != 0 { if ss.aliases == nil { ss.aliases = map[Atom]*Stream{} } diff --git a/engine/term_test.go b/engine/term_test.go index 7eea5db..4971429 100644 --- a/engine/term_test.go +++ b/engine/term_test.go @@ -48,13 +48,13 @@ func TestCompareAtomic(t *testing.T) { {a: &y{}, t: NewVariable(), o: 1}, {a: &y{}, t: NewFloatFromInt64(0), o: 1}, {a: &y{}, t: Integer(0), o: 1}, - {a: &y{}, t: Atom(0), o: 1}, + {a: &y{}, t: Atom{}, o: 1}, {a: &y{}, t: &x{}, o: 1}, {a: &y{val: 1}, t: &y{val: 0}, cmp: cmp, o: 1}, {a: &y{val: 0}, t: &y{val: 0}, cmp: cmp, o: 0}, {a: &y{val: 0}, t: &y{val: 1}, cmp: cmp, o: -1}, {a: &y{}, t: &z{}, o: -1}, - {a: &y{}, t: Atom(0).Apply(Integer(0)), o: -1}, + {a: &y{}, t: Atom{}.Apply(Integer(0)), o: -1}, } for _, tt := range tests { diff --git a/engine/vm.go b/engine/vm.go index c77ad61..dbd64e6 100644 --- a/engine/vm.go +++ b/engine/vm.go @@ -287,6 +287,7 @@ func (vm *VM) ResetEnv() { value: rootContext, }, } + //atomTable.atoms = map[string]Atom{} } func (vm *VM) getProcedure(p procedureIndicator) (procedure, bool) { From 598d3e7c1daa31db0ec789f5decb51ea5a3fb073 Mon Sep 17 00:00:00 2001 From: Benjamin DENEUX Date: Fri, 19 Jul 2024 10:39:41 +0200 Subject: [PATCH 2/3] refactor: transform atom from uint64 to string --- engine/atom.go | 24 +++++------------------- engine/builtin.go | 5 ++--- engine/compound.go | 4 ++-- engine/parser.go | 28 ++++++++++++++-------------- engine/stream.go | 4 ++-- engine/term_test.go | 4 ++-- engine/vm.go | 1 - 7 files changed, 27 insertions(+), 43 deletions(-) diff --git a/engine/atom.go b/engine/atom.go index dfd3273..eadeeb5 100644 --- a/engine/atom.go +++ b/engine/atom.go @@ -1,13 +1,10 @@ package engine import ( - "crypto/sha256" - "encoding/binary" "fmt" "io" "regexp" "strings" - "unicode/utf8" ) var ( @@ -199,26 +196,15 @@ var ( ) // Atom is a prolog atom. -type Atom struct { - value uint64 - name string -} +type Atom string // NewAtom interns the given string and returns an Atom. func NewAtom(name string) Atom { - // A one-char atom is just a rune. - if r, n := utf8.DecodeLastRuneInString(name); r != utf8.RuneError && n == len(name) { - return Atom{uint64(r), name} - } - - hashBytes := sha256.Sum256([]byte(name)) - hashUint64 := binary.BigEndian.Uint64(hashBytes[:8]) - - return Atom{hashUint64, name} + return Atom(name) } func NewAtomRune(v rune) Atom { - return Atom{uint64(v), string(v)} + return Atom(v) } // WriteTerm outputs the Atom to an io.Writer. @@ -227,7 +213,7 @@ func (a Atom) WriteTerm(w io.Writer, opts *WriteOptions, _ *Env) error { openClose := (opts.left != (operator{}) || opts.right != (operator{})) && opts.getOps().defined(a) if openClose { - if opts.left.name.value != 0 && opts.left.specifier.class() == operatorClassPrefix { + if opts.left.name != "" && opts.left.specifier.class() == operatorClassPrefix { _, _ = ew.Write([]byte(" ")) } _, _ = ew.Write([]byte("(")) @@ -279,7 +265,7 @@ func (a Atom) Compare(t Term, env *Env) int { } func (a Atom) String() string { - return a.name + return string(a) } // Apply returns a Compound which Functor is the Atom and args are the arguments. If the arguments are empty, diff --git a/engine/builtin.go b/engine/builtin.go index b3ccd46..8df0193 100644 --- a/engine/builtin.go +++ b/engine/builtin.go @@ -1685,12 +1685,11 @@ func PutChar(vm *VM, streamOrAlias, char Term, k Cont, env *Env) *Promise { case Variable: return Error(InstantiationError(env)) case Atom: - if c.value > utf8.MaxRune { + r, n := utf8.DecodeLastRuneInString(c.String()) + if r == utf8.RuneError || n != len(c.String()) { return Error(typeError(validTypeCharacter, c, env)) } - r := rune(c.value) - switch _, err := s.WriteRune(r); { case errors.Is(err, errWrongIOMode): return Error(permissionError(operationOutput, permissionTypeStream, streamOrAlias, env)) diff --git a/engine/compound.go b/engine/compound.go index 9a66d3d..dedd81d 100644 --- a/engine/compound.go +++ b/engine/compound.go @@ -204,7 +204,7 @@ func writeCompoundOpInfix(w io.Writer, c Compound, opts *WriteOptions, env *Env, (opts.right != operator{} && r >= opts.right.priority) if openClose { - if opts.left.name.value != 0 && opts.left.specifier.class() == operatorClassPrefix { + if opts.left.name != "" && opts.left.specifier.class() == operatorClassPrefix { _, _ = fmt.Fprint(&ew, " ") } _, _ = fmt.Fprint(&ew, "(") @@ -494,7 +494,7 @@ func pair(k, v Term) Term { } func tuple(args ...Term) Term { - return Atom{}.Apply(args...) + return Atom("").Apply(args...) } type charList string diff --git a/engine/parser.go b/engine/parser.go index c668991..1a8fd3c 100644 --- a/engine/parser.go +++ b/engine/parser.go @@ -463,13 +463,13 @@ func (p *Parser) op(maxPriority Integer) (Atom, error) { if p.current().kind == tokenCloseList { p.backup() } - return Atom{}, errNoOp + return "", errNoOp case atomEmptyBlock: p.backup() if p.current().kind == tokenCloseCurly { p.backup() } - return Atom{}, errNoOp + return "", errNoOp default: return a, nil } @@ -477,7 +477,7 @@ func (p *Parser) op(maxPriority Integer) (Atom, error) { t, err := p.next() if err != nil { - return Atom{}, err + return "", err } switch t.kind { case tokenComma: @@ -489,7 +489,7 @@ func (p *Parser) op(maxPriority Integer) (Atom, error) { } p.backup() - return Atom{}, errExpectation + return "", errExpectation } func (p *Parser) term0(maxPriority Integer) (Term, error) { @@ -571,7 +571,7 @@ func (p *Parser) term0Atom(maxPriority Integer) (Term, error) { return nil, errExpectation } - if p.placeholder.value != 0 && t == p.placeholder { + if p.placeholder != "" && t == p.placeholder { if len(p.args) == 0 { return nil, errPlaceholder } @@ -616,13 +616,13 @@ func (p *Parser) atom() (Atom, error) { t, err := p.next() if err != nil { - return Atom{}, err + return "", err } switch t.kind { case tokenOpenList: t, err := p.next() if err != nil { - return Atom{}, err + return "", err } switch t.kind { case tokenCloseList: @@ -630,12 +630,12 @@ func (p *Parser) atom() (Atom, error) { default: p.backup() p.backup() - return Atom{}, errExpectation + return "", errExpectation } case tokenOpenCurly: t, err := p.next() if err != nil { - return Atom{}, err + return "", err } switch t.kind { case tokenCloseCurly: @@ -643,7 +643,7 @@ func (p *Parser) atom() (Atom, error) { default: p.backup() p.backup() - return Atom{}, errExpectation + return "", errExpectation } case tokenDoubleQuotedList: switch p.doubleQuotes { @@ -651,18 +651,18 @@ func (p *Parser) atom() (Atom, error) { return NewAtom(unDoubleQuote(t.val)), nil default: p.backup() - return Atom{}, errExpectation + return "", errExpectation } default: p.backup() - return Atom{}, errExpectation + return "", errExpectation } } func (p *Parser) name() (Atom, error) { t, err := p.next() if err != nil { - return Atom{}, err + return "", err } switch t.kind { case tokenLetterDigit, tokenGraphic, tokenSemicolon, tokenCut: @@ -671,7 +671,7 @@ func (p *Parser) name() (Atom, error) { return NewAtom(unquote(t.val)), nil default: p.backup() - return Atom{}, errExpectation + return "", errExpectation } } diff --git a/engine/stream.go b/engine/stream.go index e49b4f6..4f171c0 100644 --- a/engine/stream.go +++ b/engine/stream.go @@ -359,7 +359,7 @@ func (s *Stream) properties() []Term { ps = append(ps, atomOutput) } - if s.alias.value != 0 { + if s.alias != "" { ps = append(ps, atomAlias.Apply(s.alias)) } @@ -510,7 +510,7 @@ type streams struct { } func (ss *streams) add(s *Stream) { - if s.alias.value != 0 { + if s.alias != "" { if ss.aliases == nil { ss.aliases = map[Atom]*Stream{} } diff --git a/engine/term_test.go b/engine/term_test.go index 4971429..3af7a53 100644 --- a/engine/term_test.go +++ b/engine/term_test.go @@ -48,13 +48,13 @@ func TestCompareAtomic(t *testing.T) { {a: &y{}, t: NewVariable(), o: 1}, {a: &y{}, t: NewFloatFromInt64(0), o: 1}, {a: &y{}, t: Integer(0), o: 1}, - {a: &y{}, t: Atom{}, o: 1}, + {a: &y{}, t: Atom(""), o: 1}, {a: &y{}, t: &x{}, o: 1}, {a: &y{val: 1}, t: &y{val: 0}, cmp: cmp, o: 1}, {a: &y{val: 0}, t: &y{val: 0}, cmp: cmp, o: 0}, {a: &y{val: 0}, t: &y{val: 1}, cmp: cmp, o: -1}, {a: &y{}, t: &z{}, o: -1}, - {a: &y{}, t: Atom{}.Apply(Integer(0)), o: -1}, + {a: &y{}, t: Atom("").Apply(Integer(0)), o: -1}, } for _, tt := range tests { diff --git a/engine/vm.go b/engine/vm.go index dbd64e6..c77ad61 100644 --- a/engine/vm.go +++ b/engine/vm.go @@ -287,7 +287,6 @@ func (vm *VM) ResetEnv() { value: rootContext, }, } - //atomTable.atoms = map[string]Atom{} } func (vm *VM) getProcedure(p procedureIndicator) (procedure, bool) { From cdbf28dbc9a97199ed3b5cc65febd1931abbebe1 Mon Sep 17 00:00:00 2001 From: Benjamin DENEUX Date: Fri, 19 Jul 2024 14:34:23 +0200 Subject: [PATCH 3/3] test: test reset env vm --- engine/vm_test.go | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/engine/vm_test.go b/engine/vm_test.go index e8d3548..1fb733d 100644 --- a/engine/vm_test.go +++ b/engine/vm_test.go @@ -286,3 +286,25 @@ func TestProcedureIndicator_Apply(t *testing.T) { assert.Nil(t, c) }) } + +func TestVM_ResetEnv(t *testing.T) { + var vm VM + varCounter = 10 + varContext = NewVariable() + rootContext = NewAtom("non-root") + rootEnv = &Env{ + binding: binding{ + key: newEnvKey(varContext), + value: NewAtom("non-root"), + }, + } + + t.Run("Reset environment", func(t *testing.T) { + vm.ResetEnv() + + assert.Equal(t, int64(1), varCounter) // 1 because NewVariable() is called in ResetEnv() + assert.Equal(t, "root", rootContext.String()) + assert.Equal(t, newEnvKey(varContext), rootEnv.binding.key) + assert.Equal(t, NewAtom("root"), rootEnv.binding.value) + }) +}