o symbols

- reworked symbols package

debugger/debugger.go

@@ -261,7 +261,7 @@ func (dbg *Debugger) loadCartridge(cartridgeFilename string) error {
 	symtable, err := symbols.ReadSymbolsFile(cartridgeFilename)
 	if err != nil {
 		dbg.print(console.Error, "%s", err)
-		symtable = symbols.StandardSymbolTable()
+		// continuing because symtable is always valid even if err non-nil
 	}
 
 	err = dbg.disasm.FromMemory(dbg.vcs.Mem.Cart, symtable)

debugger/memory.go

@@ -4,7 +4,6 @@ import (
 	"fmt"
 	"gopher2600/errors"
 	"gopher2600/hardware/memory"
-	"gopher2600/hardware/memory/addresses"
 	"gopher2600/symbols"
 	"strconv"
 	"strings"
@@ -75,9 +74,9 @@ func (mem memoryDebug) mapAddress(address interface{}, cpuRead bool) *addressInf
 	var symbolTable map[uint16]string
 
 	if cpuRead {
-		symbolTable = (*mem.symtable).ReadSymbols
+		symbolTable = (*mem.symtable).Read.Symbols
 	} else {
-		symbolTable = addresses.Write
+		symbolTable = (*mem.symtable).Write.Symbols
 	}
 
 	switch address := address.(type) {

disassembly/disassembly.go

@@ -63,15 +63,13 @@ func (dsm *Disassembly) Dump(output io.Writer) {
 // disassembly from the supplied cartridge filename. - useful for one-shot
 // disassemblies, like the gopher2600 "disasm" mode
 func FromCartrige(cartridgeFilename string) (*Disassembly, error) {
-	// ignore errors caused by loading of symbols table
-	symtable, err := symbols.ReadSymbolsFile(cartridgeFilename)
-	if err != nil {
-		symtable = symbols.StandardSymbolTable()
-	}
+	// ignore errors caused by loading of symbols table - we always get a
+	// standard symbols table even in the event of an error
+	symtable, _ := symbols.ReadSymbolsFile(cartridgeFilename)
 
 	cart := memory.NewCart()
 
-	err = cart.Attach(cartridgeFilename)
+	err := cart.Attach(cartridgeFilename)
 	if err != nil {
 		return nil, errors.NewFormattedError(errors.DisasmError, err)
 	}

hardware/cpu/result/instruction.go

@@ -45,10 +45,6 @@ type Instruction struct {
 	Bug string
 }
 
-func (result Instruction) String() string {
-	return result.GetString(symbols.StandardSymbolTable(), StyleFlagAddress|StyleFlagSymbols)
-}
-
 // GetString returns a human readable version of InstructionResult, addresses
 // replaced with symbols if supplied symbols argument is not null. prefer this
 // function to implicit calls to String()
@@ -68,7 +64,7 @@ func (result Instruction) GetString(symtable *symbols.Table, style Style) string
 	if result.Final && style.Has(StyleFlagAddress) {
 		programCounter = fmt.Sprintf("0x%04x", result.Address)
 		if symtable != nil && style.Has(StyleFlagLocation) {
-			if v, ok := symtable.Locations[result.Address]; ok {
+			if v, ok := symtable.Locations.Symbols[result.Address]; ok {
 				label = v
 			}
 		}
@@ -147,23 +143,23 @@ func (result Instruction) GetString(symtable *symbols.Table, style Style) string
 						pc.Add(idx, false)
 
 						// -- look up mock program counter value in symbol table
-						if v, ok := symtable.Locations[pc.ToUint16()]; ok {
+						if v, ok := symtable.Locations.Symbols[pc.ToUint16()]; ok {
 							operand = v
 						}
 
 					} else {
-						if v, ok := symtable.Locations[idx]; ok {
+						if v, ok := symtable.Locations.Symbols[idx]; ok {
 							operand = v
 						}
 					}
 				case definitions.Read:
-					if v, ok := symtable.ReadSymbols[idx]; ok {
+					if v, ok := symtable.Read.Symbols[idx]; ok {
 						operand = v
 					}
 				case definitions.Write:
 					fallthrough
 				case definitions.RMW:
-					if v, ok := symtable.WriteSymbols[idx]; ok {
+					if v, ok := symtable.Write.Symbols[idx]; ok {
 						operand = v
 					}
 				}

symbols/file.go

@@ -0,0 +1,122 @@
+package symbols
+
+import (
+	"fmt"
+	"gopher2600/errors"
+	"gopher2600/hardware/memory/addresses"
+	"io/ioutil"
+	"os"
+	"path"
+	"sort"
+	"strconv"
+	"strings"
+	"unicode"
+)
+
+// ReadSymbolsFile initialises a symbols table from the symbols file for the
+// specified cartridge
+//
+// Table instance will always be valid even if error is returned. for example,
+// if the symbols file cannot be opened the symbols file will still contain the
+// canonical vcs symbols file
+//
+// currently, only symbols files generated by DASM are supported
+func ReadSymbolsFile(cartridgeFilename string) (*Table, error) {
+	table := new(Table)
+	table.Locations = newTable()
+	table.Read = newTable()
+	table.Write = newTable()
+
+	// prioritise symbols with reference symbols for the VCS.
+	//
+	// deferred function because we want to do this in all instances, even if
+	// there is an error with the symbols file.
+	defer func() {
+		for k, v := range addresses.Read {
+			table.Read.add(k, v, true)
+		}
+		for k, v := range addresses.Write {
+			table.Write.add(k, v, true)
+		}
+
+		sort.Sort(table.Locations)
+		sort.Sort(table.Read)
+		sort.Sort(table.Write)
+
+		// find max symbol width
+		table.MaxLocationWidth = table.Locations.maxWidth
+		if table.Read.maxWidth > table.Write.maxWidth {
+			table.MaxSymbolWidth = table.Read.maxWidth
+		} else {
+			table.MaxSymbolWidth = table.Write.maxWidth
+		}
+	}()
+
+	// if this is the empty cartridge then this error is expected. return
+	// the empty symbol table
+	if cartridgeFilename == "" {
+		return table, nil
+	}
+
+	// try to open symbols file
+	symFilename := cartridgeFilename
+	ext := path.Ext(symFilename)
+
+	// try to figure out the case of the file extension
+	if ext == ".BIN" {
+		symFilename = fmt.Sprintf("%s.SYM", symFilename[:len(symFilename)-len(ext)])
+	} else {
+		symFilename = fmt.Sprintf("%s.sym", symFilename[:len(symFilename)-len(ext)])
+	}
+
+	sf, err := os.Open(symFilename)
+	if err != nil {
+		return table, errors.NewFormattedError(errors.SymbolsFileUnavailable, cartridgeFilename)
+	}
+	defer func() {
+		_ = sf.Close()
+	}()
+
+	sym, err := ioutil.ReadAll(sf)
+	if err != nil {
+		return nil, errors.NewFormattedError(errors.SymbolsFileError, err)
+	}
+	lines := strings.Split(string(sym), "\n")
+
+	// find interesting lines in the symbols file and add to the Table
+	// instance.
+	for _, ln := range lines {
+		// ignore uninteresting lines
+		p := strings.Fields(ln)
+		if len(p) < 2 || p[0] == "---" {
+			continue // for loop
+		}
+
+		// get address
+		address, err := strconv.ParseUint(p[1], 16, 16)
+		if err != nil {
+			continue // for loop
+		}
+
+		// get symbol
+		symbol := p[0]
+
+		// differentiate between location and other symbols. this is a little
+		// heavy handed, but still, it's better than nothing.
+		if unicode.IsDigit(rune(symbol[0])) {
+			// if symbol begins with a number and a period then it is a location symbol
+			i := strings.Index(symbol, ".")
+			if i != -1 {
+				table.Locations.add(uint16(address), symbol[i:], false)
+			}
+		} else {
+			// every non-location symbols is both a read and write symbol.
+			// compar to canonical vcs symbols which are specific to a read or
+			// write context
+			table.Read.add(uint16(address), symbol, false)
+			table.Write.add(uint16(address), symbol, false)
+		}
+	}
+
+	return table, nil
+}

symbols/list.go

@@ -15,23 +15,17 @@ func (tab *Table) ListSymbols(output io.Writer) {
 // ListLocations outputs every location symbol used in the current ROM
 func (tab *Table) ListLocations(output io.Writer) {
 	output.Write([]byte(fmt.Sprintf("Locations\n---------\n")))
-	for i := range tab.locations {
-		output.Write([]byte(fmt.Sprintf("%#04x -> %s\n", i, tab.Locations[tab.locations[i]])))
-	}
+	output.Write([]byte(tab.Locations.String()))
 }
 
 // ListReadSymbols outputs every read symbol used in the current ROM
 func (tab *Table) ListReadSymbols(output io.Writer) {
 	output.Write([]byte(fmt.Sprintf("\nRead Symbols\n-----------\n")))
-	for i := range tab.readSymbols {
-		output.Write([]byte(fmt.Sprintf("%#04x -> %s\n", i, tab.ReadSymbols[tab.readSymbols[i]])))
-	}
+	output.Write([]byte(tab.Read.String()))
 }
 
 // ListWriteSymbols outputs every write symbol used in the current ROM
 func (tab *Table) ListWriteSymbols(output io.Writer) {
 	output.Write([]byte(fmt.Sprintf("\nWrite Symbols\n------------\n")))
-	for i := range tab.writeSymbols {
-		output.Write([]byte(fmt.Sprintf("%#04x -> %s\n", i, tab.WriteSymbols[tab.writeSymbols[i]])))
-	}
+	output.Write([]byte(tab.Write.String()))
 }

symbols/search.go

@@ -23,26 +23,20 @@ func (tab *Table) SearchSymbol(symbol string, tType TableType) (TableType, strin
 	symbolUpper := strings.ToUpper(symbol)
 
 	if tType == UnspecifiedSymTable || tType == LocationSymTable {
-		for k, v := range tab.Locations {
-			if strings.ToUpper(v) == symbolUpper {
-				return LocationSymTable, symbol, k, nil
-			}
+		if addr, ok := tab.Locations.search(symbolUpper); ok {
+			return LocationSymTable, symbol, addr, nil
 		}
 	}
 
 	if tType == UnspecifiedSymTable || tType == ReadSymTable {
-		for k, v := range tab.ReadSymbols {
-			if strings.ToUpper(v) == symbolUpper {
-				return ReadSymTable, v, k, nil
-			}
+		if addr, ok := tab.Read.search(symbolUpper); ok {
+			return ReadSymTable, symbol, addr, nil
 		}
 	}
 
 	if tType == UnspecifiedSymTable || tType == WriteSymTable {
-		for k, v := range tab.WriteSymbols {
-			if strings.ToUpper(v) == symbolUpper {
-				return WriteSymTable, v, k, nil
-			}
+		if addr, ok := tab.Write.search(symbolUpper); ok {
+			return WriteSymTable, symbol, addr, nil
 		}
 	}
 

symbols/symbols.go

@@ -1,171 +0,0 @@
-package symbols
-
-import (
-	"fmt"
-	"gopher2600/errors"
-	"gopher2600/hardware/memory/addresses"
-	"io/ioutil"
-	"os"
-	"path"
-	"sort"
-	"strconv"
-	"strings"
-	"unicode"
-)
-
-type keys []uint16
-
-// Len is the number of elements in the collection
-func (k keys) Len() int {
-	return len(k)
-}
-
-// Less reports whether the element with index i should sort before the element
-// with index j
-func (k keys) Less(i, j int) bool {
-	return k[i] < k[j]
-}
-
-// Swap swaps the elements with indexes i and j
-func (k keys) Swap(i, j int) {
-	k[i], k[j] = k[j], k[i]
-}
-
-// Table is the symbols table for the loaded programme
-type Table struct {
-	Locations    map[uint16]string
-	ReadSymbols  map[uint16]string
-	WriteSymbols map[uint16]string
-
-	// sorted keys
-	locations    keys
-	readSymbols  keys
-	writeSymbols keys
-
-	MaxLocationWidth int
-	MaxSymbolWidth   int
-}
-
-// StandardSymbolTable initialises a symbols table using the standard VCS symbols
-func StandardSymbolTable() *Table {
-	table := new(Table)
-	table.ReadSymbols = addresses.Read
-	table.WriteSymbols = addresses.Write
-	table.genMaxWidths()
-	return table
-}
-
-// ReadSymbolsFile initialises a symbols table from the symbols file for the
-// specified cartridge
-func ReadSymbolsFile(cartridgeFilename string) (*Table, error) {
-	table := new(Table)
-	table.Locations = make(map[uint16]string)
-	table.ReadSymbols = make(map[uint16]string)
-	table.WriteSymbols = make(map[uint16]string)
-	table.locations = make(keys, 0)
-	table.readSymbols = make(keys, 0)
-	table.writeSymbols = make(keys, 0)
-
-	// prioritise symbols with reference symbols for the VCS. do this in all
-	// instances, even if there is an error with the symbols file
-	defer func() {
-		for k, v := range addresses.Read {
-			table.ReadSymbols[k] = v
-		}
-		for k, v := range addresses.Write {
-			table.WriteSymbols[k] = v
-		}
-
-		table.genMaxWidths()
-	}()
-
-	// try to open symbols file
-	symFilename := cartridgeFilename
-	ext := path.Ext(symFilename)
-
-	// try to figure out the case of the file extension
-	if ext == ".BIN" {
-		symFilename = fmt.Sprintf("%s.SYM", symFilename[:len(symFilename)-len(ext)])
-	} else {
-		symFilename = fmt.Sprintf("%s.sym", symFilename[:len(symFilename)-len(ext)])
-	}
-
-	sf, err := os.Open(symFilename)
-	if err != nil {
-		// if this is the empty cartridge then this error is expected. return
-		// the empty symbol table
-		if cartridgeFilename == "" {
-			return table, nil
-		}
-		return nil, errors.NewFormattedError(errors.SymbolsFileUnavailable, cartridgeFilename)
-	}
-	defer func() {
-		_ = sf.Close()
-	}()
-
-	sym, err := ioutil.ReadAll(sf)
-	if err != nil {
-		return nil, errors.NewFormattedError(errors.SymbolsFileError, err)
-	}
-	lines := strings.Split(string(sym), "\n")
-
-	// create new symbols table
-	// loop over lines
-	for _, ln := range lines {
-		// ignore uninteresting lines
-		p := strings.Fields(ln)
-		if len(p) < 2 || p[0] == "---" {
-			continue // for loop
-		}
-
-		// get address
-		address, err := strconv.ParseUint(p[1], 16, 16)
-		if err != nil {
-			continue // for loop
-		}
-
-		// get symbol
-		symbol := p[0]
-
-		if unicode.IsDigit(rune(symbol[0])) {
-			// if symbol begins with a number and a period then it is a location symbol
-			i := strings.Index(symbol, ".")
-			if i == -1 {
-				continue // for loop
-			}
-			table.Locations[uint16(address)] = symbol[i:]
-			table.locations = append(table.locations, uint16(address))
-		} else {
-			// put symbol in table
-			table.ReadSymbols[uint16(address)] = symbol
-			table.WriteSymbols[uint16(address)] = symbol
-			table.readSymbols = append(table.locations, uint16(address))
-			table.writeSymbols = append(table.locations, uint16(address))
-		}
-	}
-
-	sort.Sort(table.locations)
-	sort.Sort(table.readSymbols)
-	sort.Sort(table.writeSymbols)
-
-	return table, nil
-}
-
-// find the widest location and read/write symbol
-func (tab *Table) genMaxWidths() {
-	for _, s := range tab.Locations {
-		if len(s) > tab.MaxLocationWidth {
-			tab.MaxLocationWidth = len(s)
-		}
-	}
-	for _, s := range tab.ReadSymbols {
-		if len(s) > tab.MaxSymbolWidth {
-			tab.MaxSymbolWidth = len(s)
-		}
-	}
-	for _, s := range tab.WriteSymbols {
-		if len(s) > tab.MaxSymbolWidth {
-			tab.MaxSymbolWidth = len(s)
-		}
-	}
-}

symbols/tables.go

@@ -0,0 +1,89 @@
+package symbols
+
+import (
+	"fmt"
+	"sort"
+	"strings"
+)
+
+// Table is the symbols table for the loaded programme
+type Table struct {
+	Locations *table
+	Read      *table
+	Write     *table
+
+	MaxLocationWidth int
+	MaxSymbolWidth   int
+}
+
+type table struct {
+	Symbols  map[uint16]string
+	idx      []uint16
+	maxWidth int
+}
+
+func newTable() *table {
+	tb := new(table)
+	tb.Symbols = make(map[uint16]string)
+	tb.idx = make([]uint16, 0)
+	return tb
+}
+
+func (tb table) String() string {
+	s := strings.Builder{}
+	for i := range tb.idx {
+		s.WriteString(fmt.Sprintf("%#04x -> %s\n", i, tb.Symbols[tb.idx[i]]))
+	}
+	return s.String()
+}
+
+func (tb *table) add(addr uint16, symbol string, prefer bool) {
+	// end add procedure with check for max symbol width
+	defer func() {
+		for _, s := range tb.Symbols {
+			if len(s) > tb.maxWidth {
+				tb.maxWidth = len(s)
+			}
+		}
+	}()
+
+	// check for duplicates
+	for i := range tb.idx {
+		if tb.idx[i] == addr {
+			// overwrite existing symbol with preferred symbol
+			if prefer {
+				tb.Symbols[addr] = symbol
+			}
+			return
+		}
+	}
+
+	tb.Symbols[addr] = symbol
+	tb.idx = append(tb.idx, addr)
+	sort.Sort(tb)
+}
+
+func (tb table) search(symbol string) (uint16, bool) {
+	for k, v := range tb.Symbols {
+		if strings.ToUpper(v) == symbol {
+			return k, true
+		}
+	}
+	return 0, false
+}
+
+// Len is the number of elements in the collection
+func (tb table) Len() int {
+	return len(tb.idx)
+}
+
+// Less reports whether the element with index i should sort before the element
+// with index j
+func (tb table) Less(i, j int) bool {
+	return tb.idx[i] < tb.idx[j]
+}
+
+// Swap swaps the elements with indexes i and j
+func (tb table) Swap(i, j int) {
+	tb.idx[i], tb.idx[j] = tb.idx[j], tb.idx[i]
+}