Address code review feedback: improve error handling and performance

Co-authored-by: jackfirth <[email protected]>

Author: Copilot

default-recommendations/analyzers/identifier-usage.rkt

@@ -9,7 +9,8 @@
   [identifier-usage-analyzer expansion-analyzer?]))
 
 
-(require racket/list
+(require racket/hash
+         racket/list
          racket/stream
          rebellion/collection/entry
          rebellion/streaming/transducer
@@ -159,21 +160,24 @@
   ;; Create expanded-id-table to track bound identifiers with empty usage lists
   (define table (make-expanded-id-table))
 
+  ;; Group bound identifiers by phase for efficient lookup
+  (define bound-by-phase (make-hash))
+  
   ;; Initialize all bound identifiers with empty usage lists
   (for ([id (in-stream (binding-site-identifiers labeled-stx))])
     (define id-phase (syntax-property id 'phase))
-    (expanded-id-table-set! table (expanded-identifier id id-phase) '()))
+    (define expanded-id (expanded-identifier id id-phase))
+    (expanded-id-table-set! table expanded-id '())
+    (hash-update! bound-by-phase id-phase (λ (lst) (cons expanded-id lst)) '()))
 
-  ;; For each usage, find its binding and add it to the usage list
+  ;; For each usage, find its binding within the same phase and add it to the usage list
   (for ([used-id (in-stream (usage-site-identifiers labeled-stx))])
     (define used-phase (syntax-property used-id 'phase))
-    (for ([bound-entry (in-expanded-id-table table)])
-      (define bound-expanded-id (entry-key bound-entry))
+    (define bound-at-phase (hash-ref bound-by-phase used-phase '()))
+    (for ([bound-expanded-id bound-at-phase])
       (define bound-id (expanded-identifier-syntax bound-expanded-id))
-      (define bound-phase (expanded-identifier-phase bound-expanded-id))
-      (when (and (equal? bound-phase used-phase)
-                 (free-identifier=? bound-id used-id))
-        (define current-usages (entry-value bound-entry))
+      (when (free-identifier=? bound-id used-id)
+        (define current-usages (expanded-id-table-ref table bound-expanded-id '()))
         (expanded-id-table-set! table bound-expanded-id (cons used-id current-usages)))))
 
   table)

private/expanded-id-table.rkt

@@ -9,7 +9,8 @@
  (contract-out
   [expanded-id-table? (-> any/c boolean?)]
   [make-expanded-id-table (-> expanded-id-table?)]
-  [expanded-id-table-ref (-> expanded-id-table? expanded-identifier? any/c)]
+  [expanded-id-table-ref
+   (->* (expanded-id-table? expanded-identifier?) (failure-result/c) any/c)]
   [expanded-id-table-set! (-> expanded-id-table? expanded-identifier? any/c void?)]
   [in-expanded-id-table
    (-> expanded-id-table? (sequence/c (entry/c expanded-identifier? any/c)))]))
@@ -21,6 +22,7 @@
          racket/match
          racket/sequence
          racket/stream
+         rebellion/base/result
          rebellion/collection/entry
          syntax/id-table)
 
@@ -47,9 +49,15 @@
   (expanded-id-table (make-hasheq)))
 
 
-(define (expanded-id-table-ref table id)
-  (define phase-table (hash-ref (expanded-id-table-table table) (expanded-identifier-phase id)))
-  (free-id-table-ref phase-table (expanded-identifier-syntax id)))
+(define (expanded-id-table-ref table id [failure-result (λ () (error 'expanded-id-table-ref "no mapping for ~a" id))])
+  (define phase (expanded-identifier-phase id))
+  (define stx (expanded-identifier-syntax id))
+  (define phase-table (hash-ref (expanded-id-table-table table) phase #false))
+  (if phase-table
+      (free-id-table-ref phase-table stx failure-result)
+      (if (procedure? failure-result)
+          (failure-result)
+          failure-result)))
 
 
 (define (expanded-id-table-set! table id value)