Skip to content

Commit

Permalink
Dyno: fix disambiguation between fully-generic vararg and less generi…
Browse files Browse the repository at this point in the history
…c non-vararg. (#26456)

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

```Chapel
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:

```Chapel
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
- [x] dyno tests
  • Loading branch information
DanilaFe authored Jan 6, 2025
2 parents 04e4169 + 8f2cdbe commit f884761
Show file tree
Hide file tree
Showing 2 changed files with 26 additions and 0 deletions.
7 changes: 7 additions & 0 deletions frontend/lib/resolution/disambiguation.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1735,6 +1735,13 @@ static bool isFormalInstantiatedAny(const DisambiguationCandidate& candidate,

if (qt.type() && qt.type()->isAnyType())
return true;

if (initial->untyped()->formalIsVarArgs(formalIdx) &&
qt.type() && qt.type()->isTupleType()) {
auto tt = qt.type()->toTupleType();
CHPL_ASSERT(tt && tt->isStarTuple());
return tt->starType().type() && tt->starType().type()->isAnyType();
}
}

return false;
Expand Down
19 changes: 19 additions & 0 deletions frontend/test/resolution/testVarArgs.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -834,6 +834,24 @@ var c = fn(y=5.0, "hello", 1, "test", 3.0);

}

// regression test for a bug in which two fucntions (see body of the test)
// are incorrectly considered ambiguous. The vararg-based function is less
// specific than the 'integral'-based function since the latter constraints
// the type more.
static void testGenericInstantiationDisambiguation() {
Context* context = buildStdContext();
auto program =
R"""(
proc foo(x...) do return 1.0;
proc foo(x: integral) do return 1;
var x = foo(10);
)""";

auto qt = resolveTypeOfX(context, program);
CHPL_ASSERT(qt->isIntType());
}

//
// TODO: test where-clauses:
//
Expand Down Expand Up @@ -864,6 +882,7 @@ int main(int argc, char** argv) {
testConcrete();
testCount();
testAlignment();
testGenericInstantiationDisambiguation();

printf("\nAll tests passed successfully.\n");

Expand Down

0 comments on commit f884761

Please sign in to comment.