for modules we use validate_module_contents which builds up a map of (identifier, element) pairs.
If we see an entry twice we emit an error
for everything else we pass in a list of identifiers, which is sorted then checked for duplicates
This PR removes the 2nd method, and merges all the checks into a single check_for_redefinitions function.
Additionally, we run this this before the rest of validation and exit early if there's a problem (right now we run these checks in the middle of validation).
Why?
If there's a redefinition error, a validator might see the other definition, since only one of them will be visible in the AST.
This can lead to spurious error messages about an element that's actually defined somewhere totally different.
Our tests are small, so this doesn't happen in them, but could happen in a larger project with redefinitions in it.
Currently we check for redefinitions in 2 ways:
validate_module_contentswhich builds up a map of (identifier, element) pairs. If we see an entry twice we emit an errorThis PR removes the 2nd method, and merges all the checks into a single
check_for_redefinitionsfunction. Additionally, we run this this before the rest of validation and exit early if there's a problem (right now we run these checks in the middle of validation).Why? If there's a redefinition error, a validator might see the other definition, since only one of them will be visible in the AST. This can lead to spurious error messages about an element that's actually defined somewhere totally different. Our tests are small, so this doesn't happen in them, but could happen in a larger project with redefinitions in it.