Cleaned up bitfield handling so it's future proof (OR)

internal/compiler/symboltable.go

@@ -26,23 +26,37 @@ const (
 	FlagLabelRef
 )
 
+// Symbol represents a variable, constant, or label reference
+type Symbol struct {
+	Name     string       // Variable name
+	Scope    string       // Function scope (empty for global)
+	Flags    SymbolFlags  // Type and properties
+	Value    uint16       // Initial value for variables, constant value for constants
+	AbsAddr  uint16       // Absolute address (for @ variables)
+	LabelRef string       // Label reference (for label variables)
+	Usage    uint8        // Usage tracking
+	Filename string       // Source file where declared
+	LineNo   int          // Line number in source file
+}
+
 // Usage tracking for variables
 const (
-	UsageNone uint8 = iota
+	UsageNone uint8 = 0
-	UsageUsed
+	UsageUsed uint8 = 1 << 0
+	// Future flags can be added:
+	// UsageRead      = 1 << 1
+	// UsageWritten   = 1 << 2  
+	// UsageAddress   = 1 << 3
 )
 
-// Symbol represents a variable or constant declaration
+// MarkUsed marks the symbol as used (should not be called for constants or absolutes)
-type Symbol struct {
+func (s *Symbol) MarkUsed() {
-	Name     string
+	s.Usage |= UsageUsed
-	Scope    string // empty string = global, otherwise function name
+}
-	Flags    SymbolFlags
+
-	Value    uint16 // init value or const value
+// IsUsed returns true if the symbol has been marked as used
-	AbsAddr  uint16 // if FlagAbsolute set
+func (s *Symbol) IsUsed() bool {
-	LabelRef string // if FlagLabelRef set
+	return s.Usage&UsageUsed != 0
-	Usage    uint8  // tracks variable usage (see Usage constants)
-	Filename string // file where variable was declared
-	LineNo   int    // line number where variable was declared
 }
 
 // Helper methods for Symbol
@@ -61,11 +75,6 @@ func (s *Symbol) IsAbsolute() bool        { return s.Has(FlagAbsolute) }
 func (s *Symbol) IsZeroPage() bool        { return s.Has(FlagZeroPage) }
 func (s *Symbol) IsZeroPagePointer() bool { return s.HasAll(FlagAbsolute | FlagZeroPage | FlagWord) }
 
-// MarkUsed marks the symbol as used (should not be called for constants or absolutes)
-func (s *Symbol) MarkUsed() {
-	s.Usage = UsageUsed
-}
-
 // FullName returns the fully qualified name (scope.name or just name)
 func (s *Symbol) FullName() string {
 	if s.Scope == "" {
@@ -330,12 +339,18 @@ func (st *SymbolTable) ConstantLookupFunc(currentScopes []string) func(string) (
 func (st *SymbolTable) CheckUnused() []string {
 	var warnings []string
 	for _, sym := range st.symbols {
-		// Skip constants and absolute variables
+		// Skip constants and absolute variables (they shouldn't track usage)
 		if sym.IsConst() || sym.IsAbsolute() {
+			// Sanity check: constants and absolutes should never be marked as used
+			// If they are, it's a bug in the compiler
+			if sym.IsUsed() {
+				// This would be an internal error, but we'll just skip it
+				continue
+			}
 			continue
 		}
 		// Check if variable was never used
-		if sym.Usage == UsageNone {
+		if !sym.IsUsed() {
 			// Format warning message with file and line info
 			var scopeInfo string
 			if sym.Scope != "" {

internal/compiler/symboltable_test.go

@@ -693,3 +693,381 @@ func TestGenerateHexLowercase(t *testing.T) {
 		t.Error("expected lowercase hex in variable")
 	}
 }
+
+func TestUsageTracking(t *testing.T) {
+	t.Run("Unused variable generates warning", func(t *testing.T) {
+		st := NewSymbolTable()
+		
+		// Add an unused variable
+		err := st.AddVar("unused", "", KindByte, 0, "test.c65", 10)
+		if err != nil {
+			t.Fatalf("AddVar() error = %v", err)
+		}
+		
+		// Check that it's marked as unused
+		sym := st.Get("unused")
+		if sym == nil {
+			t.Fatal("symbol not found")
+		}
+		if sym.IsUsed() {
+			t.Errorf("IsUsed() = true, want false")
+		}
+		
+		// Check that warning is generated
+		warnings := st.CheckUnused()
+		if len(warnings) != 1 {
+			t.Fatalf("CheckUnused() returned %d warnings, want 1", len(warnings))
+		}
+		
+		expected := "test.c65:10: warning: variable 'unused' declared but never used"
+		if warnings[0] != expected {
+			t.Errorf("warning = %q, want %q", warnings[0], expected)
+		}
+	})
+	
+	t.Run("Used variable doesn't generate warning", func(t *testing.T) {
+		st := NewSymbolTable()
+		
+		// Add a variable
+		err := st.AddVar("used", "", KindByte, 0, "test.c65", 20)
+		if err != nil {
+			t.Fatalf("AddVar() error = %v", err)
+		}
+		
+		// Mark it as used via Lookup (simulating actual usage)
+		sym := st.Lookup("used", []string{})
+		if sym == nil {
+			t.Fatal("symbol not found")
+		}
+		
+		// Check that it's marked as used
+		if !sym.IsUsed() {
+			t.Errorf("IsUsed() = false, want true")
+		}
+		
+		// Check that no warning is generated
+		warnings := st.CheckUnused()
+		if len(warnings) != 0 {
+			t.Errorf("CheckUnused() returned %d warnings, want 0: %v", len(warnings), warnings)
+		}
+	})
+	
+	t.Run("Constants are ignored", func(t *testing.T) {
+		st := NewSymbolTable()
+		
+		// Add a constant (should not generate warning)
+		err := st.AddConst("MAX", "", KindByte, 100, "test.c65", 30)
+		if err != nil {
+			t.Fatalf("AddConst() error = %v", err)
+		}
+		
+		// Check that no warning is generated
+		warnings := st.CheckUnused()
+		if len(warnings) != 0 {
+			t.Errorf("CheckUnused() returned %d warnings for constant, want 0: %v", len(warnings), warnings)
+		}
+		
+		// Verify constant is not marked as used (constants shouldn't track usage)
+		sym := st.Get("MAX")
+		if sym.IsUsed() {
+			t.Errorf("Constant IsUsed() = true, want false")
+		}
+	})
+	
+	t.Run("Absolute variables are ignored", func(t *testing.T) {
+		st := NewSymbolTable()
+		
+		// Add an absolute variable (should not generate warning)
+		err := st.AddAbsolute("SCREEN", "", KindWord, 0xD000, "test.c65", 40)
+		if err != nil {
+			t.Fatalf("AddAbsolute() error = %v", err)
+		}
+		
+		// Check that no warning is generated
+		warnings := st.CheckUnused()
+		if len(warnings) != 0 {
+			t.Errorf("CheckUnused() returned %d warnings for absolute variable, want 0: %v", len(warnings), warnings)
+		}
+		
+		// Verify absolute variable is not marked as used (absolutes shouldn't track usage)
+		sym := st.Get("SCREEN")
+		if sym.IsUsed() {
+			t.Errorf("Absolute variable IsUsed() = true, want false")
+		}
+	})
+	
+	t.Run("Function-scoped variables", func(t *testing.T) {
+		st := NewSymbolTable()
+		
+		// Add a global unused variable
+		err := st.AddVar("global_unused", "", KindByte, 0, "test.c65", 50)
+		if err != nil {
+			t.Fatalf("AddVar() error = %v", err)
+		}
+		
+		// Add a function-scoped unused variable
+		err = st.AddVar("local_unused", "myFunc", KindByte, 0, "test.c65", 60)
+		if err != nil {
+			t.Fatalf("AddVar() error = %v", err)
+		}
+		
+		// Add a function-scoped used variable
+		err = st.AddVar("local_used", "myFunc", KindByte, 0, "test.c65", 70)
+		if err != nil {
+			t.Fatalf("AddVar() error = %v", err)
+		}
+		st.Lookup("local_used", []string{"myFunc"})
+		
+		// Check warnings
+		warnings := st.CheckUnused()
+		if len(warnings) != 2 {
+			t.Fatalf("CheckUnused() returned %d warnings, want 2: %v", len(warnings), warnings)
+		}
+		
+		// Check warning formats
+		expectedGlobal := "test.c65:50: warning: variable 'global_unused' declared but never used"
+		expectedLocal := "test.c65:60: warning: variable 'local_unused' in function 'myFunc' declared but never used"
+		
+		foundGlobal := false
+		foundLocal := false
+		for _, w := range warnings {
+			if w == expectedGlobal {
+				foundGlobal = true
+			}
+			if w == expectedLocal {
+				foundLocal = true
+			}
+		}
+		
+		if !foundGlobal {
+			t.Errorf("Missing warning for global variable: %q", expectedGlobal)
+		}
+		if !foundLocal {
+			t.Errorf("Missing warning for local variable: %q", expectedLocal)
+		}
+	})
+	
+	t.Run("Multiple lookups don't affect usage flag", func(t *testing.T) {
+		st := NewSymbolTable()
+		
+		// Add a variable
+		err := st.AddVar("counter", "", KindByte, 0, "test.c65", 80)
+		if err != nil {
+			t.Fatalf("AddVar() error = %v", err)
+		}
+		
+		// Lookup multiple times (should only mark as used once)
+		sym1 := st.Lookup("counter", []string{})
+		sym2 := st.Lookup("counter", []string{})
+		sym3 := st.Lookup("counter", []string{})
+		
+		if sym1 != sym2 || sym2 != sym3 {
+			t.Error("Lookup should return same symbol instance")
+		}
+		
+		// Should still be marked as used (idempotent)
+		if !sym1.IsUsed() {
+			t.Errorf("IsUsed() after multiple lookups = false, want true")
+		}
+		
+		// No warnings should be generated
+		warnings := st.CheckUnused()
+		if len(warnings) != 0 {
+			t.Errorf("CheckUnused() returned %d warnings for used variable, want 0", len(warnings))
+		}
+	})
+	
+	t.Run("LookupWithoutUsage doesn't mark as used", func(t *testing.T) {
+		st := NewSymbolTable()
+		
+		// Add a variable
+		err := st.AddVar("temp", "", KindByte, 0, "test.c65", 90)
+		if err != nil {
+			t.Fatalf("AddVar() error = %v", err)
+		}
+		
+		// Use LookupWithoutUsage (validation only, not actual usage)
+		sym := st.LookupWithoutUsage("temp", []string{})
+		if sym == nil {
+			t.Fatal("symbol not found")
+		}
+		
+		// Should NOT be marked as used
+		if sym.IsUsed() {
+			t.Errorf("IsUsed() after LookupWithoutUsage = true, want false")
+		}
+		
+		// Warning should be generated
+		warnings := st.CheckUnused()
+		if len(warnings) != 1 {
+			t.Errorf("CheckUnused() returned %d warnings, want 1", len(warnings))
+		}
+	})
+	
+	t.Run("Mixed variables with usage", func(t *testing.T) {
+		st := NewSymbolTable()
+		
+		// Add various types of variables
+		st.AddVar("unused1", "", KindByte, 0, "test.c65", 100)
+		st.AddVar("used1", "", KindByte, 0, "test.c65", 101)
+		st.AddConst("CONST1", "", KindByte, 255, "test.c65", 102)
+		st.AddAbsolute("ABS1", "", KindWord, 0xC000, "test.c65", 103)
+		st.AddVar("unused2", "func1", KindWord, 0, "test.c65", 104)
+		st.AddVar("used2", "func1", KindWord, 0, "test.c65", 105)
+		
+		// Mark some as used
+		st.Lookup("used1", []string{})
+		st.Lookup("used2", []string{"func1"})
+		
+		// Check warnings
+		warnings := st.CheckUnused()
+		if len(warnings) != 2 {
+			t.Fatalf("CheckUnused() returned %d warnings, want 2: %v", len(warnings), warnings)
+		}
+		
+		// Verify correct warnings
+		expected1 := "test.c65:100: warning: variable 'unused1' declared but never used"
+		expected2 := "test.c65:104: warning: variable 'unused2' in function 'func1' declared but never used"
+		
+		found1 := false
+		found2 := false
+		for _, w := range warnings {
+			if w == expected1 {
+				found1 = true
+			}
+			if w == expected2 {
+				found2 = true
+			}
+		}
+		
+		if !found1 {
+			t.Errorf("Missing warning: %q", expected1)
+		}
+		if !found2 {
+			t.Errorf("Missing warning: %q", expected2)
+		}
+	})
+	
+	t.Run("Label references", func(t *testing.T) {
+		st := NewSymbolTable()
+		
+		// Add a label reference variable
+		err := st.AddLabel("handler", "", "irq_vector", "test.c65", 110)
+		if err != nil {
+			t.Fatalf("AddLabel() error = %v", err)
+		}
+		
+		// Label references are word variables that reference labels
+		// They should be treated like regular variables for usage tracking
+		sym := st.Get("handler")
+		if !sym.Has(FlagLabelRef) {
+			t.Error("Expected FlagLabelRef")
+		}
+		if !sym.IsWord() {
+			t.Error("Label reference should be word")
+		}
+		
+		// Since we haven't used it, it should generate a warning
+		warnings := st.CheckUnused()
+		if len(warnings) != 1 {
+			t.Errorf("CheckUnused() returned %d warnings for label reference, want 1", len(warnings))
+		}
+		
+		// Mark it as used
+		st.Lookup("handler", []string{})
+		
+		// Now no warning should be generated
+		warnings = st.CheckUnused()
+		if len(warnings) != 0 {
+			t.Errorf("CheckUnused() returned %d warnings for used label reference, want 0", len(warnings))
+		}
+	})
+	
+	t.Run("LookupConstant doesn't mark as used", func(t *testing.T) {
+		st := NewSymbolTable()
+		
+		// Add a regular variable
+		err := st.AddVar("var1", "", KindByte, 0, "test.c65", 120)
+		if err != nil {
+			t.Fatalf("AddVar() error = %v", err)
+		}
+		
+		// Add a constant
+		err = st.AddConst("CONST1", "", KindByte, 100, "test.c65", 121)
+		if err != nil {
+			t.Fatalf("AddConst() error = %v", err)
+		}
+		
+		// Lookup constant (should not mark variable as used)
+		val, found := st.LookupConstant("CONST1", []string{})
+		if !found {
+			t.Fatal("constant not found")
+		}
+		if val != 100 {
+			t.Errorf("constant value = %d, want 100", val)
+		}
+		
+		// LookupConstant on non-constant variable (should not mark as used)
+		val, found = st.LookupConstant("var1", []string{})
+		if found {
+			t.Error("LookupConstant should return false for non-constant")
+		}
+		
+		// Check that variable is still unused
+		sym := st.Get("var1")
+		if sym.IsUsed() {
+			t.Errorf("IsUsed() after LookupConstant = true, want false")
+		}
+		
+		// Warning should be generated for the variable
+		warnings := st.CheckUnused()
+		if len(warnings) != 1 {
+			t.Errorf("CheckUnused() returned %d warnings, want 1", len(warnings))
+		}
+		
+		expected := "test.c65:120: warning: variable 'var1' declared but never used"
+		if warnings[0] != expected {
+			t.Errorf("warning = %q, want %q", warnings[0], expected)
+		}
+	})
+	
+	t.Run("Lookup doesn't mark constants or absolutes as used", func(t *testing.T) {
+		st := NewSymbolTable()
+		
+		// Add a constant
+		err := st.AddConst("MAX", "", KindByte, 255, "test.c65", 130)
+		if err != nil {
+			t.Fatalf("AddConst() error = %v", err)
+		}
+		
+		// Add an absolute variable
+		err = st.AddAbsolute("VIC", "", KindWord, 0xD000, "test.c65", 131)
+		if err != nil {
+			t.Fatalf("AddAbsolute() error = %v", err)
+		}
+		
+		// Lookup constant (should not mark as used)
+		symConst := st.Lookup("MAX", []string{})
+		if symConst == nil {
+			t.Fatal("constant not found")
+		}
+		if symConst.IsUsed() {
+			t.Error("constant should not be marked as used after Lookup")
+		}
+		
+		// Lookup absolute variable (should not mark as used)
+		symAbs := st.Lookup("VIC", []string{})
+		if symAbs == nil {
+			t.Fatal("absolute variable not found")
+		}
+		if symAbs.IsUsed() {
+			t.Error("absolute variable should not be marked as used after Lookup")
+		}
+		
+		// No warnings should be generated
+		warnings := st.CheckUnused()
+		if len(warnings) != 0 {
+			t.Errorf("CheckUnused() returned %d warnings for constants/absolutes, want 0", len(warnings))
+		}
+	})
+}