Skip to content
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

Dyno: fix disambiguation between fully-generic vararg and less generic non-vararg. #26456

Merged
merged 2 commits into from
Jan 6, 2025

Conversation

DanilaFe
Copy link
Contributor

@DanilaFe DanilaFe commented Jan 2, 2025

This fixes an issue discovered in domain module code, in which the following program was considered to be ambiguous:

proc foo(x...) {}
proc foo(x: integral) {}

foo(10);

This program is not actually ambiguous, which can be seen by making an appeal to "desugaring" of the vararg version to its non-vararg form:

proc foo(x) {}
proc foo(x: integral) {}

foo(10);

The first version of foo is fully-generic, while the second is more constrained (it requires an integral type). As a result, the second version is more specific. Dyno didn't detect this because generic vararg formal are given generic tuple types, while the "is fully generic" check was looking for any type. Adjust this by detecting vararg formals and changing the check accordingly.

Reviewed @arezaii -- thanks!

Testing

  • dyno tests

@DanilaFe DanilaFe mentioned this pull request Jan 2, 2025
2 tasks
@DanilaFe DanilaFe merged commit f884761 into chapel-lang:main Jan 6, 2025
8 checks passed
DanilaFe added a commit that referenced this pull request Jan 8, 2025
Closes Cray/chapel-private#6923.

Depends on #26454,
#26455 and
#26456 because this causes
more Domain methods to be resolved.

The issue was in resolving code like this:

```Chapel
class C {
  var x: int;
  var y: string;

  iter type myIter() {
    yield 3;
    yield 5;
    yield 7;
    yield 11;
  }
}

for i in C.myIter() do
  writeln(i);
```

Previously, dyno incorrectly set `isMethod` to be false, which made the
`this` formal for methods to be incorrectly set. This PR adjusts this
behavior to correctly track method calls when resolving iterator
overloads.

Reviewed by @benharsh -- thanks!

## Testing
- [x] dyno tests
- [x] paratest
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