Skip to content

Dyno: implement field access promotion #26945

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Mar 22, 2025

Conversation

DanilaFe
Copy link
Contributor

@DanilaFe DanilaFe commented Mar 19, 2025

Closes https://github.com/Cray/chapel-private/issues/7227.

This PR implements field access promotion; that is, we can now resolve code like this:

record pair {
  var first: int;
  var second: real;
}
var A: [1..10] pair;
var B = A.first;
var C = A.second;

I ran into issues implementing this naively, and took a look at how production does this. However, as I discovered in #26938, production doesn't implement this feature properly, either!

In both Dyno and production, fields are accessed using their "getters". From there, promotion works the same as it does with anything else (by finding methods that apply to the element type, and creating wrappers). The crux of the issue is that we need to "discover" the getters in the current scope. Production does this incorrectly: it searches the scope in which the promotion would occur, and it searches the scope of the receiver. However, it does not search the scopes of the scalar elements. This leads to the bug discoveed in 26938.

The proper way to handle this situation is to find and check the scopes of the scalar types. However, to do this, we need to know the scalar types! This requires us to resole chpl__promotionType. The PR does just that: if we can't find regular candidates, and if we can't find forwarded methods, for parenless method calls, it computes the receiver's scalar type and searches it for receiver scopes.

This leads to problem with recursion. In general, we always try to firs resolve code like x.foo() as (x.foo)(), which means we try to find foo as a field. This often does not succeed (particularly if x.foo is a function). However, x.foo matches all the conditions in the preceding paragraph. This means that, naively, we end up with recursion as follows;

  1. We attempt to resolve a type's chpl__promotionType method
  2. This method exists, and has a method access (e.g., this.isRectangular())
  3. We attempt to resolve this.isRectangular as a field, and we don't find it.
  4. We try to find this.isRectangular in the scalar types, which requires resolving chpl__promotionType.

This PR takes the "naive" approach that we take elsewhere: using isQueryRunning. Notably, there is an open concern with isQueryRunning as a whole, well-documented in #26459. However, a "proper" resolution to this issue would be very involved or very performance-heavy.

Reviewed by @benharsh -- thanks!

Testing

  • dyno tests
  • paratest

@DanilaFe DanilaFe force-pushed the field-access-promotion branch from 5c8f737 to f69d231 Compare March 19, 2025 20:58
Signed-off-by: Danila Fedorin <[email protected]>
@DanilaFe DanilaFe marked this pull request as ready for review March 19, 2025 23:45
@benharsh benharsh self-requested a review March 20, 2025 20:32
@DanilaFe DanilaFe merged commit 501182d into chapel-lang:main Mar 22, 2025
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants