silver icon indicating copy to clipboard operation
silver copied to clipboard

Fix for "Ambiguous reference to type 'fully:Qualified'" errors.

Open remexre opened this issue 3 years ago • 2 comments

Changes

In a situation like:

grammar foo;
nt Foo;

grammar bar;
exports foo;

grammar baz;
import foo;
import bar;
type Baz = foo:Foo;

Despite fully qualifying foo:Foo, we still get an ambiguous-reference error, since we could be getting foo:Foo from the import of foo or from the import of bar, since bar exports foo.

We fix this by sorting and uniq-ing the results of environment queries, using the full name as a key. It should never (right? right??) be the case that:

  1. there are multiple items with the same fullName in the environment
  2. the only place this is checked is the lookup of that fullName

so this ought to be a sound fix.

top.dcls should always have exactly one element in a non-error case, so the sort and uniq calls should be cheap. In theory, only the uniq is necessary, but I guess this could improve error message quality in some other case too. A <=1-element list is the base case of the current implementation of sort, though, so yeah, cheap.

Documentation

Wrote a good description of the issue, also present as a commit message so that when/if it breaks, git blame is useful for more than assigning blame.

remexre avatar Feb 01 '22 09:02 remexre

Updated this to do the same for searchEnvAll; I'm less sure about this, but propagate is otherwise broken on these multiple-import-paths productions.

For this one, I figure the O(n lg n) of uniq(sort(...)) over nub's O(n^2) matters, though I didn't measure this.

remexre avatar Feb 01 '22 16:02 remexre

There's still a weird behavior where non-location annotations seem to be getting duplicated, but I don't think it's critical for my use-case, so unless it's obvious to you where it is, it's fine to go on the "maybe next hackathon" backburner.

remexre avatar Feb 01 '22 16:02 remexre