16357, “lydia-duncan”, “Replacing versus supplementing traversal of private uses and imports, continued”, “2020-09-08T22:35:58Z”
This is a continuation of #16183, intended to give folks a fresh entry point to the discussion so that we can finalize it.
Basically, we are trying to determine the set of rules used to determine whether a field or method can be resolved for an instance of a type that isn’t necessarily visible to the current scope. We’ve had various solutions for this problem over the years, but we always seem to run into a case where a user expects to be able to call a method on an instance they’ve obtained and can’t. Our latest solution causes us to always go into the scope where a type is defined to search for methods and fields on it.
The most recent previous solution involved ignoring the privacy and limitations of uses and imports when resolving method calls (and things like operators on the type). The main drawback of this approach is that it means private and the other limitations are a lie w.r.t. methods and fields, which could be confusing to users. The new solution handles many of the cases that motivated us to perform that previous traversal, so we wanted to determine whether this meant that we should stop performing that traversal as well or whether it was best to continue to do both. In looking at the impact of no longer performing that traversal, only one test failure gave me pause. The following test worked under the previous solution, but is not handled by going to the point where the type is defined (because we are trying to resolve a secondary method that was defined in another module entirely):
test/visibility/only/canSeeSecondaryMethod2.chpl:
module OuterModule {
use secondaryMethod only Foo; // The module secondaryMethod defines type `Foo`
use Extender only Foo;
// Verifies that you can define a method in a module outside of the original
// type definition, and by specifying that type in your 'only' list, access
// it.
var a = new owned Foo(7);
writeln(a.innerMethod(3)); // Should be 21
writeln(a.newMethod(1)); // Should be 8
module Extender {
use secondaryMethod only Foo;
proc Foo.newMethod(z: int) {
return field + z;
}
proc unrelated() {
writeln("I'm unrelated to Foo!");
}
}
}
It seems like something that would cause a user frustration if it did not work.
Related side bar (included for completeness but not necessary for understanding):
Even more alternatively - prior to the previous solution, we would track symbols associated with a type, so that when the type was mentioned in an except or only list, we would allow or block its methods appropriately. Now obviously that wasn’t sufficient on its own (or we wouldn’t have had to consider the more recent changes). However, we could choose to restore that solution while also stopping the traversal of uses that I described earlier, to ensure the above test still continues to pass.
Ideally, I’d like us to determine a path forward that minimizes the possibility of users asking for a method to be available that wouldn’t be available otherwise. While I would prefer to stop ignoring aspects of use and import statements in order to find these methods, I’d much prefer not having to alter the traversal strategy again after this change because we forgot another pattern.
So, in summary, our options as I see them are:
A. Continue to support both going to the type’s definition point and traversing any uses and imports we find, regardless of their privacy or limitations
B. Go to the type’s definition point, but return to respecting the privacy and limitations of uses and imports, accepting that this test will fail (and that a user will probably try to argue with us about it)
C. Go to the type’s definition point, return to respecting the privacy and limitations of uses and imports, and restore tracking of symbols associated with a type name (and hope that is enough that users won’t find other edge cases we haven’t thought of)
If there’s another option you can think of, please let me know.