Fail loudly on duplicate scoped identifiers in SymbolConverter#4570
Fail loudly on duplicate scoped identifiers in SymbolConverter#4570pepone merged 2 commits intoicerpc:mainfrom
Conversation
pepone
force-pushed
the
fix/named-type-duplicate-detection
branch
from
93b2712 to
4c1c45b
Compare
There was a problem hiding this comment.
Pull request overview
This PR addresses issue #4500 by ensuring duplicate fully-scoped identifiers in SymbolConverter no longer get silently ignored during symbol indexing, making name collisions (or unexpected slicec output) immediately visible.
Changes:
- Replace
_named.TryAdd(...)with_named.Add(...)so duplicate keys throw instead of being silently dropped. - Add an explanatory comment documenting why duplicates are treated as a hard failure.
| // Scoped IDs are warned to be unique by the compiler, we can safely use them as dictionary keys. | ||
| _named.Add($"{moduleScope}::{id}", (file, symbol)); | ||
| } | ||
| } |
There was a problem hiding this comment.
Dictionary.Add will throw an ArgumentException on duplicates, but the message typically won’t include which SliceFile(s) produced the conflicting symbols. Since this is intended to diagnose slicec/multi-file collisions, consider checking for an existing entry first and throwing with additional context (duplicate id + both file paths/symbol kinds) to make the failure easier to debug.
InsertCreativityHere
left a comment
InsertCreativityHere
left a comment
There was a problem hiding this comment.
Looks good to me!
Co-authored-by: Joe George <joe@externl.com>
Fix #4500