diff --git a/internal/compiler/symboltable.go b/internal/compiler/symboltable.go index 8de5ced..ffb99f7 100644 --- a/internal/compiler/symboltable.go +++ b/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 - UsageUsed + UsageNone uint8 = 0 + 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 -type Symbol struct { - Name string - Scope string // empty string = global, otherwise function name - Flags SymbolFlags - Value uint16 // init value or const value - AbsAddr uint16 // if FlagAbsolute set - LabelRef string // if FlagLabelRef set - Usage uint8 // tracks variable usage (see Usage constants) - Filename string // file where variable was declared - LineNo int // line number where variable was declared +// MarkUsed marks the symbol as used (should not be called for constants or absolutes) +func (s *Symbol) MarkUsed() { + s.Usage |= UsageUsed +} + +// IsUsed returns true if the symbol has been marked as used +func (s *Symbol) IsUsed() bool { + return s.Usage&UsageUsed != 0 } // 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 != "" { diff --git a/internal/compiler/symboltable_test.go b/internal/compiler/symboltable_test.go index 1533fd0..3613f23 100644 --- a/internal/compiler/symboltable_test.go +++ b/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)) + } + }) +}