refactor variables manager to own pkg

internal/arch/disasm.go

@@ -11,9 +11,6 @@ import (
 type Disasm interface {
 	// AddAddressToParse adds an address to the list to be processed if the address has not been processed yet.
 	AddAddressToParse(address, context, from uint16, currentInstruction Instruction, isABranchDestination bool)
-	// AddVariableReference adds a variable reference if the opcode is accessing
-	// the given address directly by reading or writing.
-	AddVariableReference(addressReference, usageAddress uint16, opcode Opcode, forceVariableUsage bool)
 	// Cart returns the loaded cartridge.
 	Cart() *cartridge.Cartridge
 	// ChangeAddressRangeToCodeAsData sets a range of code address to code as
@@ -29,6 +26,8 @@ type Disasm interface {
 	JumpEngine() JumpEngine
 	// Logger returns the logger.
 	Logger() *log.Logger
+	// Mapper returns the mapper.
+	Mapper() Mapper
 	// OffsetInfo returns the offset information for the given address.
 	OffsetInfo(address uint16) *Offset
 	// Options returns the disassembler options.
@@ -45,10 +44,20 @@ type Disasm interface {
 	SetHandlers(handlers program.Handlers)
 	// SetVectorsStartAddress sets the start address of the vectors.
 	SetVectorsStartAddress(address uint16)
+	// Variables returns the variable manager.
+	Variables() VariableManager
 }
 
+// ConstantManager manages constants in the disassembled program.
 type ConstantManager interface {
 	// ReplaceParameter replaces the parameter of an instruction by a constant name
 	// if the address of the instruction is found in the constants map.
 	ReplaceParameter(address uint16, opcode Opcode, paramAsString string) (string, bool)
 }
+
+// VariableManager manages variables in the disassembled program.
+type VariableManager interface {
+	// AddReference adds a variable reference if the opcode is accessing
+	// the given address directly by reading or writing.
+	AddReference(dis Disasm, addressReference, usageAddress uint16, opcode Opcode, forceVariableUsage bool)
+}

internal/arch/m6502/parser.go

@@ -118,7 +118,7 @@ func (ar *Arch6502) replaceParamByAlias(dis arch.Disasm, address uint16, opcode
 		return changedParamAsString
 	}
 
-	dis.AddVariableReference(addressReference, address, opcode, forceVariableUsage)
+	dis.Variables().AddReference(dis, addressReference, address, opcode, forceVariableUsage)
 	return paramAsString
 }
 

internal/arch/mapper.go

@@ -1,16 +1,18 @@
 package arch
 
-type Bank interface {
-	AddVariableUsage(ref any)
+type Mapper interface {
+	GetMappedBank(address uint16) MappedBank
+	GetMappedBankIndex(address uint16) uint16
 }
 
 type MappedBank interface {
-	Bank() Bank
+	ID() int
 	OffsetInfo(address uint16) *Offset
 }
 
 type BankReference struct {
 	Mapped  MappedBank
 	Address uint16
-	Index   uint16
+	ID      int
+	Index   uint16 // index in the bank
 }

internal/bank.go

@@ -15,24 +15,13 @@ const (
 type bank struct {
 	prg []byte
 
-	variables     map[uint16]*variable
-	usedVariables map[uint16]struct{}
-
 	offsets []*arch.Offset
 }
 
-func (b *bank) AddVariableUsage(ref any) {
-	varInfo := ref.(*variable)
-	b.variables[varInfo.address] = varInfo
-	b.usedVariables[varInfo.address] = struct{}{}
-}
-
 func newBank(prg []byte) *bank {
 	b := &bank{
-		prg:           prg,
-		variables:     map[uint16]*variable{},
-		usedVariables: map[uint16]struct{}{},
-		offsets:       make([]*arch.Offset, len(prg)),
+		prg:     prg,
+		offsets: make([]*arch.Offset, len(prg)),
 	}
 	for i := range b.offsets {
 		b.offsets[i] = &arch.Offset{}
@@ -53,6 +42,7 @@ func (dis *Disasm) initializeBanks(prg []byte) {
 		i += size
 
 		dis.constants.AddBank()
+		dis.vars.AddBank()
 	}
 }
 

internal/disasm.go

@@ -13,6 +13,7 @@ import (
 	"github.com/retroenv/nesgodisasm/internal/jumpengine"
 	"github.com/retroenv/nesgodisasm/internal/options"
 	"github.com/retroenv/nesgodisasm/internal/program"
+	"github.com/retroenv/nesgodisasm/internal/vars"
 	"github.com/retroenv/nesgodisasm/internal/writer"
 	"github.com/retroenv/retrogolib/arch/nes/cartridge"
 	"github.com/retroenv/retrogolib/arch/nes/codedatalog"
@@ -39,11 +40,9 @@ type Disasm struct {
 	codeBaseAddress     uint16 // codebase address of the cartridge, it is not always 0x8000
 	vectorsStartAddress uint16
 
+	constants  *consts.Consts
 	jumpEngine *jumpengine.JumpEngine
-
-	constants     *consts.Consts
-	variables     map[uint16]*variable
-	usedVariables map[uint16]struct{}
+	vars       *vars.Vars
 
 	branchDestinations map[uint16]struct{} // set of all addresses that are branched to
 
@@ -68,9 +67,8 @@ func New(ar arch.Architecture, logger *log.Logger, cart *cartridge.Cartridge,
 		logger:                      logger,
 		options:                     options,
 		cart:                        cart,
+		vars:                        vars.New(ar),
 		fileWriterConstructor:       fileWriterConstructor,
-		variables:                   map[uint16]*variable{},
-		usedVariables:               map[uint16]struct{}{},
 		branchDestinations:          map[uint16]struct{}{},
 		offsetsToParseAdded:         map[uint16]struct{}{},
 		offsetsParsed:               map[uint16]struct{}{},
@@ -109,8 +107,8 @@ func (dis *Disasm) Process(mainWriter io.Writer, newBankWriter assembler.NewBank
 	}
 
 	dis.processData()
-	if err := dis.processVariables(); err != nil {
-		return nil, err
+	if err := dis.vars.Process(dis); err != nil {
+		return nil, fmt.Errorf("processing variables: %w", err)
 	}
 	dis.constants.ProcessConstants()
 	dis.processJumpDestinations()
@@ -171,11 +169,21 @@ func (dis *Disasm) Constants() arch.ConstantManager {
 	return dis.constants
 }
 
+// Variables returns the variable manager.
+func (dis *Disasm) Variables() arch.VariableManager {
+	return dis.vars
+}
+
 // JumpEngine returns the jump engine.
 func (dis *Disasm) JumpEngine() arch.JumpEngine {
 	return dis.jumpEngine
 }
 
+// Mapper returns the mapper.
+func (dis *Disasm) Mapper() arch.Mapper {
+	return dis.mapper
+}
+
 // converts the internal disassembly representation to a program type that will be used by
 // the chosen assembler output instance to generate the asm file.
 func (dis *Disasm) convertToProgram() (*program.Program, error) {
@@ -198,11 +206,7 @@ func (dis *Disasm) convertToProgram() (*program.Program, error) {
 		}
 
 		dis.constants.SetBankConstants(bnkIndex, prgBank)
-
-		for address := range bnk.usedVariables {
-			varInfo := bnk.variables[address]
-			prgBank.Variables[varInfo.name] = address
-		}
+		dis.vars.SetBankVariables(bnkIndex, prgBank)
 
 		setBankName(prgBank, bnkIndex, len(dis.banks))
 		setBankVectors(bnk, prgBank)
@@ -211,11 +215,7 @@ func (dis *Disasm) convertToProgram() (*program.Program, error) {
 	}
 
 	dis.constants.SetProgramConstants(app)
-
-	for address := range dis.usedVariables {
-		varInfo := dis.variables[address]
-		app.Variables[varInfo.name] = address
-	}
+	dis.vars.SetProgramVariables(app)
 
 	crc32q := crc32.MakeTable(crc32.IEEE)
 	app.Checksums.PRG = crc32.Checksum(dis.cart.PRG, crc32q)

internal/mapper.go

@@ -6,11 +6,18 @@ import (
 	"github.com/retroenv/nesgodisasm/internal/arch"
 )
 
+var _ arch.MappedBank = mappedBank{}
+
 type mappedBank struct {
 	bank      *bank
+	id        int
 	dataStart int
 }
 
+func (m mappedBank) ID() int {
+	return m.id
+}
+
 type mapper struct {
 	addressShifts  int
 	bankWindowSize int
@@ -36,14 +43,15 @@ func newMapper(banks []*bank, prgSize int) (*mapper, error) {
 	}
 
 	bankNumber := 0
-	for _, bnk := range banks {
+	for bankIndex, bnk := range banks {
 		if len(bnk.prg)%bankWindowSize != 0 {
 			return nil, fmt.Errorf("invalid bank alignment for bank size %d", len(bnk.prg))
 		}
 
 		for pointer := 0; pointer < len(bnk.prg); pointer += bankWindowSize {
 			mapped := mappedBank{
 				bank:      bnk,
+				id:        bankIndex,
 				dataStart: pointer,
 			}
 			m.banks[bankNumber] = mapped
@@ -69,13 +77,13 @@ func (m *mapper) setMappedBank(address uint16, bank mappedBank) {
 	m.mapped[bankWindow] = bank
 }
 
-func (m *mapper) getMappedBank(address uint16) arch.MappedBank {
+func (m *mapper) GetMappedBank(address uint16) arch.MappedBank {
 	bankWindow := address >> m.addressShifts
 	mapped := m.mapped[bankWindow]
 	return mapped
 }
 
-func (m *mapper) getMappedBankIndex(address uint16) uint16 {
+func (m *mapper) GetMappedBankIndex(address uint16) uint16 {
 	index := int(address) % bankWindowSize
 	return uint16(index)
 }
@@ -108,10 +116,6 @@ func (m mappedBank) OffsetInfo(index uint16) *arch.Offset {
 	return offsetInfo
 }
 
-func (m mappedBank) Bank() arch.Bank {
-	return m.bank
-}
-
 // log2 computes the binary logarithm of x, rounded up to the next integer.
 func log2(i int) int {
 	var n, p int

internal/parser.go

@@ -123,10 +123,11 @@ func (dis *Disasm) AddAddressToParse(address, context, from uint16,
 	if isABranchDestination {
 		if from > 0 {
 			bankRef := arch.BankReference{
-				Mapped:  dis.mapper.getMappedBank(from),
+				Mapped:  dis.mapper.GetMappedBank(from),
 				Address: from,
-				Index:   dis.mapper.getMappedBankIndex(from),
+				Index:   dis.mapper.GetMappedBankIndex(from),
 			}
+			bankRef.ID = bankRef.Mapped.ID()
 			offsetInfo.BranchFrom = append(offsetInfo.BranchFrom, bankRef)
 		}
 		dis.branchDestinations[address] = struct{}{}

internal/parser_test.go

@@ -67,7 +67,7 @@ func TestChangeOffsetRangeToData(t *testing.T) {
 			}
 			test.Input(offsets)
 
-			m := disasm.mapper.getMappedBank(0x8000)
+			m := disasm.mapper.GetMappedBank(0x8000)
 			mapped := m.(mappedBank)
 			mapped.bank.offsets = offsets
 			disasm.ChangeAddressRangeToCodeAsData(0x8000, data)