google-research / dex-lang

Research language for array processing in the Haskell/ML family
BSD 3-Clause "New" or "Revised" License
1.58k stars 106 forks source link

Issue #323 #1254

Closed normanrink closed 1 year ago

normanrink commented 1 year ago

Allow method definitions to refer to methods that appear earlier in the class definition.

Prior to this PR, when defining a new instance of a class, the method definitions in that instance cannot use other class methods at the same instance, not even if other methods appear (lexically) earlier in the instance definition. This PR lifts this restriction as follows.

When defining a method in an instance definition, the body of the method may refer to other methods at the same instance (i.e. the instance that is currently being defined), provided the methods referred to appear (lexically) earlier in the class definition than the method that is currently being defined.

Note that this PR does not allow instance method definitions to refer to arbitrary class methods at the same instance. In particular, inside a method definition one cannot use the method at the same instance that is currently being defined. Allowing this would lead to non-termination in the compiler's simplification pass.

To detect (and disallow) potential non-termination, this PR extends ClassArrows with a list of method indices idxs. The meaning of these indices idxs is that any instance dictionary dict passed to a ClassArrow with idxs must have access to the class methods with indices idxs at instance dict. In particular, an instance that is still under construction can only grant access to method indices of those methods that have already been defined in the instance under construction.

Note that the central place where ClassArrows with specific method indices are created is in makeMethodGetter. ClassArrows that are created in other places generally require Full access to the class's method indices. In particular, when a ClassArrow ?=> results from source code that the user has entered, this arrow ?=> will require Full access to the class's methods.

axch commented 1 year ago

The Python test breakage is unrelated, and https://github.com/google-research/dex-lang/pull/1255 should fix it.

normanrink commented 1 year ago

You currently specify the dictionary requirements using an arbitrary list of method indices. But the list will always be [0..n] for some n. So perhaps we could use a more precise type and just using n directly. That is, we'd have data RequiredMethodAccess = Full | Partial Int instead of Full | Partial [Int]. Then isMethodAccessAllowedBy just has to check i < n. Any reason not to do that?

I agree that it would be simpler to have data RequiredMethodAccess = Full | Partial Int, where the Int parameter on Partial encodes the number of methods to which access is required (instead of having a [Int] that encodes method indices). I am happy to make this change.

The motivation for having a list of method indices stems from the following potential extension of this PR. With the changes in this PR, when the user writes a function f with formal (class/interface/dictionary) argument [d:MyInterface a], the signature of f will have a ClassArrow Full (in pseudo-code, f : ... MyInterface a ?=> ...), regardless of whether the body of f actually needs access to all methods of d. A suitable analysis of the body of f could determine that access is only needed to a subset of methods of d. In this case, the signature of f could be rewritten with a ClassArrow (Partial idxs), where idxs contains the indices of methods of d that are actually used in the body of f.

If at some point in time the extension outlined in the previous paragraph is indeed desired, it should be easy to revert to data RequiredMethodAccess = Full | Partial [Int] (or, better yet, data RequiredMethodAccess = Full | Partial (Set Int). Hence, in the interest of simplicity, it makes sense to change this PR to use the suggested data RequiredMethodAccess = Full | Partial Int.