From 3302a73f1d189d21e54c53adb8f90ecb3e895a40 Mon Sep 17 00:00:00 2001 From: Danila Fedorin Date: Thu, 21 Nov 2024 13:37:48 -0800 Subject: [PATCH 1/2] Skip promotion for functions that return params or types Signed-off-by: Danila Fedorin --- .../lib/resolution/resolution-queries.cpp | 22 +++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/frontend/lib/resolution/resolution-queries.cpp b/frontend/lib/resolution/resolution-queries.cpp index d8f5ca6186b4..d81e1cef8b10 100644 --- a/frontend/lib/resolution/resolution-queries.cpp +++ b/frontend/lib/resolution/resolution-queries.cpp @@ -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 || + fn->returnIntent() == uast::Function::PARAM)) { + return false; + } + const UntypedFnSignature* untypedSignature = sig->untyped(); return untypedSignature->name() != USTR("these") && @@ -2044,8 +2053,6 @@ 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(); @@ -2053,6 +2060,8 @@ ApplicabilityResult instantiateSignature(ResolutionContext* rc, 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 @@ -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; From 3b51bd2edd1c518d8e168f79d7df91bc80f084fc Mon Sep 17 00:00:00 2001 From: Danila Fedorin Date: Thu, 21 Nov 2024 13:44:56 -0800 Subject: [PATCH 2/2] Add regression test for new bug Signed-off-by: Danila Fedorin --- frontend/test/resolution/testPromotion.cpp | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/frontend/test/resolution/testPromotion.cpp b/frontend/test/resolution/testPromotion.cpp index 04bc8b5fbe4a..7eba51b0902b 100644 --- a/frontend/test/resolution/testPromotion.cpp +++ b/frontend/test/resolution/testPromotion.cpp @@ -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). @@ -437,5 +457,7 @@ int main() { testTertiaryMethod(); + regressionTestRecursivePromotionTypeBug(); + return 0; }