chapel-lang / chapel

a Productive Parallel Programming Language
https://chapel-lang.org
Other
1.75k stars 410 forks source link

Dyno: Fix init= for generic types initialized from different types #24929

Closed benharsh closed 2 weeks ago

benharsh commented 2 weeks ago

This PR updates the init= implementation to correctly instantiate the declaration when a record is initialized from a type other than that record. For example, something like var x : MyWrapperType(?) = 42;.

There are two parts to this PR:

1) Updating InitResolver to call updateResolverVisibleReceiverType for each field. This will account for any potential substitutions needed to fully instantiate the type, including fields like var x : T;.

2) In Resolver::getTypeForDecl, using the computed instantiated type from the init= to set the type of the declaration.

A couple of tests are added to lock in this behavior.

Testing:

DanilaFe commented 2 weeks ago

Updating InitResolver to call updateResolverVisibleReceiverType for each field. This will account for any potential substitutions needed to fully instantiate the type, including fields like var x : T;.

Huh, I wouldn't imagine that we need a substitution for a field that references another (type field). So if there's a type T; before the var x in your code, I don't think we should add a new substitution for the var. But if we didn't need one, then the original call to updateResolverVisibleReceiverType (that would fire on main for T) would take care of the needed substitutions. That said, I do see why you would need substitutions for fields such as var x; -- is this perhaps what you meant?

benharsh commented 2 weeks ago

Currently for types like this:

record R {
  type T;
  var x : T;
}

We require a substitution for both fields because the type of x depends on another substitution. I'm open to finding another way of representing the type of x, but it's not a small change.

DanilaFe commented 2 weeks ago

We require a substitution for both fields because the type of x depends on another substitution. I'm open to finding another way of representing the type of x, but it's not a small change.

Huh, I find this odd. I could've sworn that we don't do this. My knee-jerk reaction is that we should move away from that, but not in this PR.