bytecodealliance
Fix foreign interface dependencies which are disabled by feature-gating
A foreign imported interface can be missing from pkg.interfaces either because it does not exist, or because it was disabled by feature-gating.
Previously, the parser failed in both cases, when it should only fail when it does not exist, and should succeed when the references to it are disabled by feature-gating.
By checking the set of unresolved worlds' imports and exports for references to this interface, it is possible to determine if they contain any references to the interface, and whether or not they were disabled by feature gating.
If they are all disabled, then it does not matter that the interface is absent from pkg.interfaces, and the code should not fail. If any references are active however, then this is an error as the code will not be able to find the interface in pkg.interfaces.
If no references exist at all, then no combination of feature enabling/disabling would satisfy the import/use statement which requires the interface, and the code should fail.
This fixes issue 2285 and a unit test was added to cover this case.
Hi @alexcrichton , yeah my initial approach was to just push None when the interface was not found in pkg.interfaces, however it caused some unit tests to fail as it results in successfully parsing some packages which import nonexistent interfaces. That's what led me to create the is_gated closure as a form of basic validation that the interface actually exists in some form. I could not see another place where it would be more appropriate to check the interface actually exists however I may be overlooking something as I am not super familiar with the code.
Just to confirm, we definitely want to fail in the case where a nonexistent interface is imported but not used, right?
(the unit tests which failed my initial approach were "tests/ui/parse-fail/bad-pkg3", "tests/ui/parse-fail/bad-pkg2" and "tests/ui/parse-fail/use-world")
Oof ok yeah that's pretty tricky. Those tests should indeed continue to keep failing and I see how "just push None" would result in those tests passing. I unfortunately don't see an easy solution to that...
Would it be possible to update how include is processed instead? That's sort of where the culprit lies here as well I think, and that'd be a more focused solution and I think less brittle than this current iteration. (I'm not exactly sure what this route would look like though or if it's viable)
hi, yeah i can do some research in that area, thanks for the pointer! if i can find a more "principled" way of fixing this then i will update this PR
i think i am in the right area with this latest change, however i am going to try and add some bookkeepping to keep track of the stability of different imports of the same dependency. i will mark as draft while i do this
Latest change records the stability of each import of the foreign dependency, and a placeholder is pushed if and only if imports exist, and they are all feature gated