chapel-lang / chapel

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

Fix overloaded parnless as base expr #26275

Closed DanilaFe closed 5 days ago

DanilaFe commented 6 days ago

Closes https://github.com/chapel-lang/chapel/issues/26263.

The issue in question is that we get an internal error while invoking a return-intent-overloaded parenless procedure.

record R {}
var r = new R();
var myConstant = 1.0;

proc R.foo {
  return myConstant;
}
proc R.foo ref {
  return myConstant;
}

r.foo();

At the surface level, this is caused by the code that attempts to do return intent overloading; it assumes that the overloaded call is a part of a list. When a field is being invoked, like r.foo(), the overloaded call (r.foo) is actually the base expression, which is not in any list.

I think it's conceivable for this combination of features to be used in user code; if myConstant wasn't an integer but actually a record/class with proc this defined on it, r.foo() would invoke that proc this, and that seems reasonable enough. So, my approach was to make this case "work", up to the type-correctness of invoking the proc this.

To that end, this PR adds some extra checking to handle the case where a return-intent-overloaded call is the base expression of another call. This required two primary changes; the insertAfter diff is the first, and the partialParent->baseExpr = contextCall; is the other.

This gets us further, but code for invoking proc this doesn't expect the base expression to be a ContextCall / overloaded call either. This PR adjusts that as well.

Reviewed by @lydia-duncan -- thanks!

Testing

bradcray commented 5 days ago

Awesome, thanks Daniel!