From 5f0a65728e863013abd86b9da94076ba7646f282 Mon Sep 17 00:00:00 2001 From: Ben Harshbarger Date: Wed, 29 Jan 2025 19:13:23 -0800 Subject: [PATCH 1/6] Fix off-by-one error with de/serialize formals Signed-off-by: Ben Harshbarger --- frontend/lib/resolution/default-functions.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/frontend/lib/resolution/default-functions.cpp b/frontend/lib/resolution/default-functions.cpp index 2a01ee0e7102..9c6ccfeee5a1 100644 --- a/frontend/lib/resolution/default-functions.cpp +++ b/frontend/lib/resolution/default-functions.cpp @@ -654,11 +654,11 @@ generateDeSerialize(Context* context, const CompositeType* compType, ufsFormals.push_back( UntypedFnSignature::FormalDetail(UniqueString::get(context, channel), UntypedFnSignature::DK_NO_DEFAULT, - genFn->formal(0))); + genFn->formal(1))); ufsFormals.push_back( UntypedFnSignature::FormalDetail(UniqueString::get(context, deSerializer), UntypedFnSignature::DK_NO_DEFAULT, - genFn->formal(1))); + genFn->formal(2))); // build the untyped signature auto ufs = UntypedFnSignature::get(context, From 195c8117ea054e452623d0260c456b0eb96d9278 Mon Sep 17 00:00:00 2001 From: Ben Harshbarger Date: Wed, 29 Jan 2025 19:13:55 -0800 Subject: [PATCH 2/6] Avoid crash when trying to resolve body of extern function for return type Signed-off-by: Ben Harshbarger --- frontend/lib/resolution/return-type-inference.cpp | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/frontend/lib/resolution/return-type-inference.cpp b/frontend/lib/resolution/return-type-inference.cpp index f996ec00632c..3592ddb46cd6 100644 --- a/frontend/lib/resolution/return-type-inference.cpp +++ b/frontend/lib/resolution/return-type-inference.cpp @@ -1299,9 +1299,14 @@ static bool helpComputeReturnType(ResolutionContext* rc, } // if there are no returns with a value, use void return type - if (fn->linkage() == Decl::EXTERN && fn->returnType() == nullptr) { - result = QualifiedType(QualifiedType::CONST_VAR, VoidType::get(context)); - return true; + if (fn->linkage() == Decl::EXTERN) { + if (fn->returnType() == nullptr) { + result = QualifiedType(QualifiedType::CONST_VAR, VoidType::get(context)); + return true; + } else { + result = QualifiedType(QualifiedType::CONST_VAR, ErroneousType::get(context)); + return true; + } } else if (fnAstReturnsNonVoid(context, ast->id()) == false) { result = QualifiedType(QualifiedType::CONST_VAR, VoidType::get(context)); return true; From ea5ab6674cddac62da439bdb4168ee8c499871d5 Mon Sep 17 00:00:00 2001 From: Ben Harshbarger Date: Wed, 29 Jan 2025 19:17:26 -0800 Subject: [PATCH 3/6] Allow casts to/from both unmanaged and borrowed class types Signed-off-by: Ben Harshbarger --- frontend/lib/resolution/resolution-queries.cpp | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/frontend/lib/resolution/resolution-queries.cpp b/frontend/lib/resolution/resolution-queries.cpp index c6402cf44b6e..4f91e13560a6 100644 --- a/frontend/lib/resolution/resolution-queries.cpp +++ b/frontend/lib/resolution/resolution-queries.cpp @@ -4036,10 +4036,14 @@ static bool resolveFnCallSpecial(Context* context, // cast (borrowed class) : unmanaged auto srcClass = srcTy->toClassType(); auto dstClass = dstTy->toClassType(); - if (srcClass->decorator().isBorrowed() && - dstClass->manageableType()->isAnyClassType() && - dstClass->decorator().isUnmanaged()) { - auto decorator = ClassTypeDecorator(ClassTypeDecorator::ClassTypeDecoratorEnum::UNMANAGED); + bool isValidDst = dstClass->manageableType()->isAnyClassType() && + (dstClass->decorator().isUnmanaged() || + dstClass->decorator().isBorrowed()); + bool isValidSrc = srcClass->decorator().isBorrowed() || + srcClass->decorator().isUnmanaged(); + if (isValidDst && isValidSrc) { + auto management = ClassTypeDecorator::removeNilableFromDecorator(dstClass->decorator().val()); + auto decorator = ClassTypeDecorator(management); decorator = decorator.copyNilabilityFrom(srcClass->decorator()); auto outTy = ClassType::get(context, srcClass->manageableType(), nullptr, decorator); From 8d44c9f3be0fe3ec87061a2e557c42da7ca6c335 Mon Sep 17 00:00:00 2001 From: Ben Harshbarger Date: Wed, 29 Jan 2025 19:18:00 -0800 Subject: [PATCH 4/6] Fix assertion failure when trying to pass AnyClassType to concrete type Signed-off-by: Ben Harshbarger --- frontend/lib/resolution/can-pass.cpp | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/frontend/lib/resolution/can-pass.cpp b/frontend/lib/resolution/can-pass.cpp index 55d2dcdb794f..1ba7e4432256 100644 --- a/frontend/lib/resolution/can-pass.cpp +++ b/frontend/lib/resolution/can-pass.cpp @@ -559,7 +559,14 @@ CanPassResult CanPassResult::canPassSubtypeNonBorrowing(Context* context, instantiates = true; pass = true; } else if (actualCt->manageableType()->isAnyClassType()) { - CHPL_ASSERT(false && "probably shouldn't happen"); + // We might encounter this case when relying on 'canPass' to implement + // the various 'subtype' primitives. Types like 'borrowed class' are + // valid arguments in production, and can be found in the where-clauses + // of some basic operator implementations. + + // From the previous conditional, we know the formal isn't another + // 'any' class, so this actual cannot be passed. + pass = false; } else if (actualBct->isSubtypeOf(formalBct, converts, instantiates)) { // the basic class types are the same // or there was a subclass relationship From b5030fd3b97b5747a10223d0d8c95195e5294c94 Mon Sep 17 00:00:00 2001 From: Ben Harshbarger Date: Wed, 29 Jan 2025 19:41:08 -0800 Subject: [PATCH 5/6] Add tests for improved casting/subtype behavior for 'borrowed class' Signed-off-by: Ben Harshbarger --- frontend/test/resolution/testCast.cpp | 22 +++++++++++++++++-- .../test/resolution/testSubtypePrimitives.cpp | 10 +++++++++ 2 files changed, 30 insertions(+), 2 deletions(-) diff --git a/frontend/test/resolution/testCast.cpp b/frontend/test/resolution/testCast.cpp index 475655c2e894..e2cfce5aec0d 100644 --- a/frontend/test/resolution/testCast.cpp +++ b/frontend/test/resolution/testCast.cpp @@ -471,15 +471,24 @@ static void test45() { var f = new owned Foo(); var b : borrowed Foo = f.borrow(); var x = b:unmanaged; + + var y = x:borrowed; )"""; - auto xInit = resolveTypeOfXInit(context, program); + auto types = resolveTypesOfVariables(context, program, {"x", "y"}); + + auto xInit = types["x"]; assert(xInit.type()); auto ct = xInit.type()->toClassType(); assert(ct); assert(ct->decorator().isUnmanaged()); assert(!ct->decorator().isUnknownNilability()); assert(ct->decorator().isNonNilable()); + + auto yt = types["y"].type()->toClassType(); + assert(yt->decorator().isBorrowed()); + assert(!yt->decorator().isUnknownNilability()); + assert(yt->decorator().isNonNilable()); } // Nilable @@ -494,15 +503,24 @@ static void test45() { var f = new owned Foo(); var b : borrowed Foo? = f.borrow(); var x = b:unmanaged; + + var y = x:borrowed; )"""; - auto xInit = resolveTypeOfXInit(context, program); + auto types = resolveTypesOfVariables(context, program, {"x", "y"}); + + auto xInit = types["x"]; assert(xInit.type()); auto ct = xInit.type()->toClassType(); assert(ct); assert(ct->decorator().isUnmanaged()); assert(!ct->decorator().isUnknownNilability()); assert(ct->decorator().isNilable()); + + auto yt = types["y"].type()->toClassType(); + assert(yt->decorator().isBorrowed()); + assert(!yt->decorator().isUnknownNilability()); + assert(yt->decorator().isNilable()); } } diff --git a/frontend/test/resolution/testSubtypePrimitives.cpp b/frontend/test/resolution/testSubtypePrimitives.cpp index bf340a6fd297..c8d193ca2da8 100644 --- a/frontend/test/resolution/testSubtypePrimitives.cpp +++ b/frontend/test/resolution/testSubtypePrimitives.cpp @@ -664,6 +664,15 @@ static void test20() { }); } +static void test21() { + testPrimitive("is_subtype", { + { "borrowed class", "borrowed class?", shouldReturnFalse }, + { "borrowed class?", "borrowed class", shouldReturnTrue }, + { "borrowed Parent", "borrowed class", shouldReturnFalse }, + { "borrowed class", "borrowed Parent", shouldReturnTrue }, + }); +} + int main() { test1(); test2(); @@ -685,4 +694,5 @@ int main() { test18(); test19(); test20(); + test21(); } From fcfa248e22372388d8357292aa6bba80af60a8a7 Mon Sep 17 00:00:00 2001 From: Ben Harshbarger Date: Fri, 31 Jan 2025 10:41:14 -0800 Subject: [PATCH 6/6] Improve comments, adjust return type to something more appropriate Signed-off-by: Ben Harshbarger --- frontend/lib/resolution/resolution-queries.cpp | 3 ++- frontend/lib/resolution/return-type-inference.cpp | 5 ++++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/frontend/lib/resolution/resolution-queries.cpp b/frontend/lib/resolution/resolution-queries.cpp index 4f91e13560a6..b77439e288f6 100644 --- a/frontend/lib/resolution/resolution-queries.cpp +++ b/frontend/lib/resolution/resolution-queries.cpp @@ -4033,7 +4033,8 @@ static bool resolveFnCallSpecial(Context* context, exprTypeOut = QualifiedType::makeParamString(context, oss.str()); return true; } else if (srcTy->isClassType() && dstTy->isClassType()) { - // cast (borrowed class) : unmanaged + // cast (borrowed class) : unmanaged, + // and (unmanaged class) : borrowed auto srcClass = srcTy->toClassType(); auto dstClass = dstTy->toClassType(); bool isValidDst = dstClass->manageableType()->isAnyClassType() && diff --git a/frontend/lib/resolution/return-type-inference.cpp b/frontend/lib/resolution/return-type-inference.cpp index 3592ddb46cd6..335b9436ea5d 100644 --- a/frontend/lib/resolution/return-type-inference.cpp +++ b/frontend/lib/resolution/return-type-inference.cpp @@ -1304,7 +1304,10 @@ static bool helpComputeReturnType(ResolutionContext* rc, result = QualifiedType(QualifiedType::CONST_VAR, VoidType::get(context)); return true; } else { - result = QualifiedType(QualifiedType::CONST_VAR, ErroneousType::get(context)); + // TODO: This is a workaround for a bug where the return type was + // not found for some reason. By returning a type we prevent an attempt + // to resolve the non-existent function body. + result = QualifiedType(QualifiedType::CONST_VAR, UnknownType::get(context)); return true; } } else if (fnAstReturnsNonVoid(context, ast->id()) == false) {