Skip to content

Commit

Permalink
Fix split init internal compiler error (#26311)
Browse files Browse the repository at this point in the history
[reviewed by @mppf]

Resolves #19159.  Related to #16086, but does not resolve it.

Several people ran into this internal compiler error when trying to use
tuple syntax to split-initialize some variables. This PR makes it so
that those codes now encounter a more understandable user-facing error
that at least points them to one way they could adjust their code to
avoid it.

<details>

Allow the split init temporary type to create a ref version of itself.
Make it possible to detect the ref version of the temporary type, so
that it can be replaced with the appropriate type as part of the
expected assignment. Add a new primitive to propagate that type back to
the original variable, so that we can get a split init warning at the
attempt to split initialize, which will make the error message less
confusing.

</details>

Add the tests from #19159 as futures, linked to #16086 with their new
error message. Also add a version of one of the tests with an explicit
type, to ensure that the tuple split init works when the variables being
initialized have explicit types. Updates the .bad file for
test/types/tuple/split-init/multipleInit.chpl so that it matches the new
error message.

Passed a full paratest with futures
  • Loading branch information
lydia-duncan authored Dec 3, 2024
2 parents 29efab0 + 125a5fc commit b442d78
Show file tree
Hide file tree
Showing 21 changed files with 82 additions and 13 deletions.
3 changes: 3 additions & 0 deletions compiler/AST/primitive.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -779,6 +779,9 @@ initPrimitive() {
// if the optional type is provided, it should match PRIM_INIT_VAR_SPLIT_DECL.
prim_def(PRIM_INIT_VAR_SPLIT_INIT, "init var split init", returnInfoVoid);

prim_def(PRIM_SPLIT_INIT_UPDATE_TYPE, "split init update type",
returnInfoVoid);

prim_def(PRIM_INIT_REF_DECL, "init ref decl", returnInfoVoid);

// indicates the body of the initializer is now in Phase 2
Expand Down
2 changes: 1 addition & 1 deletion compiler/AST/type.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,7 @@ const char* toString(Type* type, bool decorateAllClasses) {

if (type == NULL ||
type == dtUnknown ||
type == dtSplitInitType) {
type->getValType() == dtSplitInitType) {
retval = "<type unknown>";
} else if (type == dtAny) {
retval = "<any type>";
Expand Down
10 changes: 10 additions & 0 deletions compiler/passes/normalize.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1534,6 +1534,16 @@ static void insertDestructureStatements(Expr* S1,
S1->insertBefore(new CallExpr(PRIM_MOVE, lhsTmp, addrOf));

S2->insertBefore(new CallExpr("=", lhsTmp, nextRHS));

if (SymExpr* orig = toSymExpr(expr)) {
if (SymExpr* origInit = toSymExpr(orig->symbol()->defPoint->init)) {
if (origInit->symbol() == gSplitInit) {
S2->insertBefore(new CallExpr(PRIM_SPLIT_INIT_UPDATE_TYPE,
orig->copy(),
new SymExpr(lhsTmp)));
}
}
}
}
}
index = index + 1;
Expand Down
2 changes: 1 addition & 1 deletion compiler/resolution/ResolutionCandidate.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1021,7 +1021,7 @@ bool ResolutionCandidate::checkGenericFormals(Expr* ctx) {
if (formalIsTypeAlias == false &&
isInitThis == false &&
formal->originalIntent != INTENT_OUT &&
(actual->type == dtSplitInitType ||
(actual->type->getValType() == dtSplitInitType ||
actual->type->symbol->hasFlag(FLAG_GENERIC))) {
failingArgument = actual;
reason = RESOLUTION_CANDIDATE_ACTUAL_TYPE_NOT_ESTABLISHED;
Expand Down
3 changes: 1 addition & 2 deletions compiler/resolution/functionResolution.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -323,7 +323,6 @@ void makeRefType(Type* type) {

if (type == dtMethodToken ||
type == dtUnknown ||
type == dtSplitInitType ||
type->symbol->hasFlag(FLAG_REF) ||
type->symbol->hasFlag(FLAG_GENERIC)) {

Expand Down Expand Up @@ -3837,7 +3836,7 @@ void resolveNormalCallAdjustAssign(CallExpr* call) {

// Adjust the type for formal_temp_out before trying to resolve '='
if (SymExpr* lhsSe = toSymExpr(lhsExpr)) {
if (targetType == dtSplitInitType) {
if (targetType->getValType() == dtSplitInitType) {
targetType = srcType;
} else if (targetType->symbol->hasFlag(FLAG_GENERIC)) {
targetType = getInstantiationType(srcType, NULL, targetType, NULL,
Expand Down
18 changes: 18 additions & 0 deletions compiler/resolution/preFold.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1315,6 +1315,24 @@ static Expr* preFoldPrimOp(CallExpr* call) {
break;
}

case PRIM_SPLIT_INIT_UPDATE_TYPE: {
INT_ASSERT(call->numActuals() == 2);

SymExpr* lhs = toSymExpr(call->get(1));
SymExpr* rhs = toSymExpr(call->get(2));

INT_ASSERT(lhs != NULL);
INT_ASSERT(rhs != NULL);

if (lhs->typeInfo() == dtSplitInitType) {
if (rhs->typeInfo()->getValType() != dtSplitInitType) {
lhs->symbol()->type = rhs->typeInfo()->getValType();
}
}
retval = new CallExpr(PRIM_NOOP);
call->replace(retval);
}

case PRIM_COERCE: {
if (SymExpr* se = toSymExpr(call->get(2))) {
if (dtDomain && se->symbol() == dtDomain->symbol) {
Expand Down
3 changes: 2 additions & 1 deletion compiler/resolution/resolveFunction.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1286,7 +1286,8 @@ bool SplitInitVisitor::enterCallExpr(CallExpr* call) {
bool isOutFormal = sym->hasFlag(FLAG_FORMAL_TEMP_OUT);

SymExpr* typeSe = toSymExpr(call->get(2));
bool requireSplitInit = (typeSe->symbol() == dtSplitInitType->symbol);
bool requireSplitInit = (typeSe->symbol() ==
dtSplitInitType->symbol);

// Don't allow an out-formal to be split-init after a return because
// that would leave the out-formal uninitialized.
Expand Down
2 changes: 1 addition & 1 deletion compiler/resolution/wrappers.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2109,7 +2109,7 @@ static void handleOutIntents(FnSymbol* fn, CallExpr* call,

// If the actual argument has generic or dtSplitInitType type,
// update it to infer the type from the called function.
if (actualSe->symbol()->type == dtSplitInitType ||
if (actualSe->symbol()->type->getValType() == dtSplitInitType ||
actualSe->symbol()->type->symbol->hasFlag(FLAG_GENERIC))
actualSe->symbol()->type = formalType;

Expand Down
1 change: 1 addition & 0 deletions frontend/include/chpl/uast/prim-ops-list.h
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ PRIMITIVE_R(INIT_FIELD, "init field")
PRIMITIVE_R(INIT_VAR, "init var")
PRIMITIVE_R(INIT_VAR_SPLIT_DECL, "init var split decl")
PRIMITIVE_R(INIT_VAR_SPLIT_INIT, "init var split init")
PRIMITIVE_R(SPLIT_INIT_UPDATE_TYPE, "split init update type")
PRIMITIVE_R(INIT_REF_DECL, "init ref decl")
PRIMITIVE_R(INIT_DONE, "init done")

Expand Down
1 change: 1 addition & 0 deletions frontend/lib/resolution/prims.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1509,6 +1509,7 @@ CallResolutionResult resolvePrimCall(ResolutionContext* rc,
case PRIM_INIT_VAR:
case PRIM_INIT_VAR_SPLIT_DECL:
case PRIM_INIT_VAR_SPLIT_INIT:
case PRIM_SPLIT_INIT_UPDATE_TYPE:
case PRIM_INIT_REF_DECL:
case PRIM_INIT_DONE:
case PRIM_REDUCE:
Expand Down
3 changes: 3 additions & 0 deletions test/types/tuple/split-init/initFromAnotherTuple.bad
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
initFromAnotherTuple.chpl:2: error: 'x' is not initialized and has no type
initFromAnotherTuple.chpl:2: note: cannot find initialization point to split-init this variable
initFromAnotherTuple.chpl:3: note: 'x' is used here before it is initialized
6 changes: 6 additions & 0 deletions test/types/tuple/split-init/initFromAnotherTuple.chpl
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
var foo = (1,2);
var x,y;
(x, y) = (foo(0), foo(1));

writeln(x);
writeln(y);
2 changes: 2 additions & 0 deletions test/types/tuple/split-init/initFromAnotherTuple.future
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
feature request: allow split init via tuple syntax when type not declared
#16086
2 changes: 2 additions & 0 deletions test/types/tuple/split-init/initFromAnotherTuple.good
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
1
2
6 changes: 6 additions & 0 deletions test/types/tuple/split-init/initFromAnotherTupleTyped.chpl
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
var foo = (1,2);
var x,y: int;
(x, y) = (foo(0), foo(1));

writeln(x);
writeln(y);
2 changes: 2 additions & 0 deletions test/types/tuple/split-init/initFromAnotherTupleTyped.good
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
1
2
3 changes: 3 additions & 0 deletions test/types/tuple/split-init/initFromFunction.bad
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
initFromFunction.chpl:8: error: 'arr1' is not initialized and has no type
initFromFunction.chpl:8: note: cannot find initialization point to split-init this variable
initFromFunction.chpl:9: note: 'arr1' is used here before it is initialized
12 changes: 12 additions & 0 deletions test/types/tuple/split-init/initFromFunction.chpl
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
proc return_arrays() {
// some size defining thing only known at runtime in function
var magic_constant : int = 5;
var arr : [0..<magic_constant] int = 0;
return (arr, arr);
}

var arr1, arr2;
(arr1, arr2) = return_arrays();

writeln(arr1);
writeln(arr2);
2 changes: 2 additions & 0 deletions test/types/tuple/split-init/initFromFunction.future
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
feature request: allow split init via tuple syntax when type not declared
#16086
2 changes: 2 additions & 0 deletions test/types/tuple/split-init/initFromFunction.good
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
0 0 0 0 0
0 0 0 0 0
10 changes: 3 additions & 7 deletions test/types/tuple/split-init/multipleInit.bad
Original file line number Diff line number Diff line change
@@ -1,7 +1,3 @@
multipleInit.chpl:2: internal error: AST-PRI-IVE-0235 chpl version 1.24.0 pre-release (8b0484037d)

Internal errors indicate a bug in the Chapel compiler,
and we're sorry for the hassle. We would appreciate your reporting this bug --
please see https://chapel-lang.org/bugs.html for instructions. In the meantime,
the filename + line number above may be useful in working around the issue.

multipleInit.chpl:1: error: 'a' is not initialized and has no type
multipleInit.chpl:1: note: cannot find initialization point to split-init this variable
multipleInit.chpl:2: note: 'a' is used here before it is initialized

0 comments on commit b442d78

Please sign in to comment.