Skip to content

Commit

Permalink
Dyno: resolve bug in de/serialize generation, and other fixes (#26631)
Browse files Browse the repository at this point in the history
This PR fixes a trivial off-by-one bug with the generation of
de/serialize formals. While poking further at IO calls, I observed some
other issues with resolution and made some fixes:
- add a workaround for case when extern function return type is not
resolved
- Improve 'canPass' to handle passing ``borrowed class`` as an actual,
which can occur in ``isSubtype``
- Improves compiler-handled casting to and from unmanaged/borrowed
any-classes

This PR also adds some tests to lock in this behavior.

[reviewed-by @DanilaFe]
  • Loading branch information
benharsh authored Jan 31, 2025
2 parents e481a13 + fcfa248 commit 2261e5e
Show file tree
Hide file tree
Showing 6 changed files with 61 additions and 13 deletions.
9 changes: 8 additions & 1 deletion frontend/lib/resolution/can-pass.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions frontend/lib/resolution/default-functions.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
15 changes: 10 additions & 5 deletions frontend/lib/resolution/resolution-queries.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4033,13 +4033,18 @@ 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();
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);
Expand Down
14 changes: 11 additions & 3 deletions frontend/lib/resolution/return-type-inference.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1299,9 +1299,17 @@ 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 {
// 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) {
result = QualifiedType(QualifiedType::CONST_VAR, VoidType::get(context));
return true;
Expand Down
22 changes: 20 additions & 2 deletions frontend/test/resolution/testCast.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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());
}
}

Expand Down
10 changes: 10 additions & 0 deletions frontend/test/resolution/testSubtypePrimitives.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -685,4 +694,5 @@ int main() {
test18();
test19();
test20();
test21();
}

0 comments on commit 2261e5e

Please sign in to comment.