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 promotion infinite recursion due to type returning functions #26296

Merged
merged 2 commits into from
Nov 21, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 18 additions & 4 deletions frontend/lib/resolution/resolution-queries.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2014,7 +2014,16 @@ static QualifiedType getProperFormalType(const ResolutionResultByPostorderID& r,
return type;
}

static bool allowPromotionForSig(const TypedFnSignature* sig) {
// if the signature is a function and has an AST, fn should be that AST.
// This is used to do some other checking, like observing return intents.
static bool allowPromotionForSig(const TypedFnSignature* sig,
const uast::Function* fn) {
// Functions that return 'param' or 'type' cannot be promoted
if (fn && (fn->returnIntent() == uast::Function::TYPE ||
riftEmber marked this conversation as resolved.
Show resolved Hide resolved
fn->returnIntent() == uast::Function::PARAM)) {
return false;
}

const UntypedFnSignature* untypedSignature = sig->untyped();
return
untypedSignature->name() != USTR("these") &&
Expand Down Expand Up @@ -2044,15 +2053,15 @@ ApplicabilityResult instantiateSignature(ResolutionContext* rc,
const AggregateDecl* ad = nullptr;
const Enum* ed = nullptr;

auto canPassFn = allowPromotionForSig(sig) ? canPass : canPassScalar;

if (!untypedSignature->id().isEmpty()) {
ast = parsing::idToAst(context, untypedSignature->id());
fn = ast->toFunction();
ad = ast->toAggregateDecl();
ed = ast->toEnum();
}

auto canPassFn = allowPromotionForSig(sig, fn) ? canPass : canPassScalar;

// If we are instantiating a nested function, then its parents should
// already be fully instantiated, in order for assumptions made during
// resolution to hold (the expectation is that the bodies of nested
Expand Down Expand Up @@ -2967,7 +2976,12 @@ isInitialTypedSignatureApplicable(Context* context,
return ApplicabilityResult::failure(tfs->id(), *reasonFailed);
}

auto canPassFn = allowPromotionForSig(tfs) ? canPass : canPassScalar;
const uast::Function* fn = nullptr;
if (!tfs->untyped()->id().isEmpty()) {
auto ast = parsing::idToAst(context, tfs->untyped()->id());
if (ast) fn = ast->toFunction();
}
auto canPassFn = allowPromotionForSig(tfs, fn) ? canPass : canPassScalar;

// Next, check that the types are compatible
int numVarArgActuals = 0;
Expand Down
22 changes: 22 additions & 0 deletions frontend/test/resolution/testPromotion.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -408,6 +408,26 @@ static void testTertiaryMethod() {

}

static void regressionTestRecursivePromotionTypeBug() {
// previously, because we allowed promotion to be applied to functions
// with the type and param intents, it was possible to trigger recursive
// queries. This PR ensures that no longer happens.

auto ctx = buildStdContext();
auto context = ctx.get();

auto var = resolveTypeOfX(context,
R"""(
use OS;
var x = createSystemError(0);
)""");

assert(var);
assert(var->isClassType());
assert(var->toClassType()->basicClassType()->parentClassType() ==
CompositeType::getErrorType(context)->basicClassType());
}

int main() {
// Run tests with primary and secondary method definitions (expecting
// the same results).
Expand Down Expand Up @@ -437,5 +457,7 @@ int main() {

testTertiaryMethod();

regressionTestRecursivePromotionTypeBug();

return 0;
}