From 66825d101cc0c01b0c3a971afee90203d0893839 Mon Sep 17 00:00:00 2001 From: Lydia Duncan Date: Tue, 12 Nov 2024 11:08:56 -0800 Subject: [PATCH 1/7] Add tests and .bad files for the split init internal compiler error No future yet, mostly snapshotting while I look at something else ---- Signed-off-by: Lydia Duncan --- test/types/tuple/split-init/initFromAnotherTuple.bad | 7 +++++++ .../types/tuple/split-init/initFromAnotherTuple.chpl | 6 ++++++ .../types/tuple/split-init/initFromAnotherTuple.good | 2 ++ test/types/tuple/split-init/initFromFunction.bad | 7 +++++++ test/types/tuple/split-init/initFromFunction.chpl | 12 ++++++++++++ test/types/tuple/split-init/initFromFunction.good | 2 ++ 6 files changed, 36 insertions(+) create mode 100644 test/types/tuple/split-init/initFromAnotherTuple.bad create mode 100644 test/types/tuple/split-init/initFromAnotherTuple.chpl create mode 100644 test/types/tuple/split-init/initFromAnotherTuple.good create mode 100644 test/types/tuple/split-init/initFromFunction.bad create mode 100644 test/types/tuple/split-init/initFromFunction.chpl create mode 100644 test/types/tuple/split-init/initFromFunction.good diff --git a/test/types/tuple/split-init/initFromAnotherTuple.bad b/test/types/tuple/split-init/initFromAnotherTuple.bad new file mode 100644 index 000000000000..d0f143122d83 --- /dev/null +++ b/test/types/tuple/split-init/initFromAnotherTuple.bad @@ -0,0 +1,7 @@ +initFromAnotherTuple.chpl:3: internal error: AST-PRI-IVE-0289 chpl version 2.3.0 pre-release (868dbb73be) + +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. + diff --git a/test/types/tuple/split-init/initFromAnotherTuple.chpl b/test/types/tuple/split-init/initFromAnotherTuple.chpl new file mode 100644 index 000000000000..5afe4ec61273 --- /dev/null +++ b/test/types/tuple/split-init/initFromAnotherTuple.chpl @@ -0,0 +1,6 @@ +var foo = (1,2); +var x,y; +(x, y) = (foo(0), foo(1)); + +writeln(x); +writeln(y); diff --git a/test/types/tuple/split-init/initFromAnotherTuple.good b/test/types/tuple/split-init/initFromAnotherTuple.good new file mode 100644 index 000000000000..1191247b6d9a --- /dev/null +++ b/test/types/tuple/split-init/initFromAnotherTuple.good @@ -0,0 +1,2 @@ +1 +2 diff --git a/test/types/tuple/split-init/initFromFunction.bad b/test/types/tuple/split-init/initFromFunction.bad new file mode 100644 index 000000000000..337518e3c537 --- /dev/null +++ b/test/types/tuple/split-init/initFromFunction.bad @@ -0,0 +1,7 @@ +initFromFunction.chpl:9: internal error: AST-PRI-IVE-0289 chpl version 2.3.0 pre-release (868dbb73be) + +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. + diff --git a/test/types/tuple/split-init/initFromFunction.chpl b/test/types/tuple/split-init/initFromFunction.chpl new file mode 100644 index 000000000000..4f9699265f1c --- /dev/null +++ b/test/types/tuple/split-init/initFromFunction.chpl @@ -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.. Date: Tue, 19 Nov 2024 14:24:44 -0800 Subject: [PATCH 2/7] Allow a ref to a split-init type This avoidance was added in #16990, but it looks like the only test impacted by it is a future (and the change in behavior seems like an improvement). With this change, we no longer get the internal error, though the tests I've written still don't behave correctly ---- Signed-off-by: Lydia Duncan --- compiler/resolution/functionResolution.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/compiler/resolution/functionResolution.cpp b/compiler/resolution/functionResolution.cpp index dd9aaee8c12e..d1169c3848c1 100644 --- a/compiler/resolution/functionResolution.cpp +++ b/compiler/resolution/functionResolution.cpp @@ -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)) { From ae9890a8d2d92e58850477ad87960a93fbe04372 Mon Sep 17 00:00:00 2001 From: Lydia Duncan Date: Tue, 19 Nov 2024 15:12:16 -0800 Subject: [PATCH 3/7] Make sure our checks against the split init temporary type account for ref-ness This doesn't completely solve the problem, but it does improve the error message without breaking the case that was working (where the variables were typed). Haven't done a full paratest run to check yet, just stashing changes while I investigate something Matt said ---- Signed-off-by: Lydia Duncan --- compiler/AST/type.cpp | 2 +- compiler/resolution/ResolutionCandidate.cpp | 2 +- compiler/resolution/functionResolution.cpp | 2 +- compiler/resolution/resolveFunction.cpp | 3 ++- compiler/resolution/wrappers.cpp | 2 +- 5 files changed, 6 insertions(+), 5 deletions(-) diff --git a/compiler/AST/type.cpp b/compiler/AST/type.cpp index 2f41f3023e3b..ac28d0a11641 100644 --- a/compiler/AST/type.cpp +++ b/compiler/AST/type.cpp @@ -188,7 +188,7 @@ const char* toString(Type* type, bool decorateAllClasses) { if (type == NULL || type == dtUnknown || - type == dtSplitInitType) { + type->getValType() == dtSplitInitType) { retval = ""; } else if (type == dtAny) { retval = ""; diff --git a/compiler/resolution/ResolutionCandidate.cpp b/compiler/resolution/ResolutionCandidate.cpp index 3ef71aa3bc7c..53ce39dba70f 100644 --- a/compiler/resolution/ResolutionCandidate.cpp +++ b/compiler/resolution/ResolutionCandidate.cpp @@ -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; diff --git a/compiler/resolution/functionResolution.cpp b/compiler/resolution/functionResolution.cpp index d1169c3848c1..02afbda331a5 100644 --- a/compiler/resolution/functionResolution.cpp +++ b/compiler/resolution/functionResolution.cpp @@ -3830,7 +3830,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, diff --git a/compiler/resolution/resolveFunction.cpp b/compiler/resolution/resolveFunction.cpp index 8f880e6991d6..b71fbd906d77 100644 --- a/compiler/resolution/resolveFunction.cpp +++ b/compiler/resolution/resolveFunction.cpp @@ -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. diff --git a/compiler/resolution/wrappers.cpp b/compiler/resolution/wrappers.cpp index a18367c9d4e5..381429b20a8e 100644 --- a/compiler/resolution/wrappers.cpp +++ b/compiler/resolution/wrappers.cpp @@ -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; From be0c953558259153ef7a534fba827a48ae1127d6 Mon Sep 17 00:00:00 2001 From: Lydia Duncan Date: Fri, 22 Nov 2024 08:47:17 -0800 Subject: [PATCH 4/7] Add a new primitive to fix the type of the original variable The type of the temporary got updated with the previous commit, but then we were encountering an issue where the later use of the variables (in the writeln) was still failing to have an actual type. This commit allows us to update the type, though it still doesn't allow us to compile the program - but the error message for the user actually points to the problem and so leaves us in a better state than the internal compiler error. To do, add a new primitive, and insert that primitive when we create the addrOf call for the variable. This allows us to let the temporary get updated and then check back in for its type prior to any later uses of the variable. ---- Signed-off-by: Lydia Duncan --- compiler/AST/primitive.cpp | 3 +++ compiler/passes/normalize.cpp | 10 ++++++++++ compiler/resolution/preFold.cpp | 18 ++++++++++++++++++ frontend/include/chpl/uast/prim-ops-list.h | 1 + frontend/lib/resolution/prims.cpp | 1 + 5 files changed, 33 insertions(+) diff --git a/compiler/AST/primitive.cpp b/compiler/AST/primitive.cpp index 34e51caf2c90..5dde8e726ad3 100644 --- a/compiler/AST/primitive.cpp +++ b/compiler/AST/primitive.cpp @@ -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 diff --git a/compiler/passes/normalize.cpp b/compiler/passes/normalize.cpp index 8ce72375ff58..01a01ae8d1ad 100644 --- a/compiler/passes/normalize.cpp +++ b/compiler/passes/normalize.cpp @@ -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; diff --git a/compiler/resolution/preFold.cpp b/compiler/resolution/preFold.cpp index fe96db091153..10572a3fccba 100644 --- a/compiler/resolution/preFold.cpp +++ b/compiler/resolution/preFold.cpp @@ -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) { diff --git a/frontend/include/chpl/uast/prim-ops-list.h b/frontend/include/chpl/uast/prim-ops-list.h index 5860f5f3a2ad..f109d03f83f2 100644 --- a/frontend/include/chpl/uast/prim-ops-list.h +++ b/frontend/include/chpl/uast/prim-ops-list.h @@ -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") diff --git a/frontend/lib/resolution/prims.cpp b/frontend/lib/resolution/prims.cpp index c98a4a6d5dde..06fc14916779 100644 --- a/frontend/lib/resolution/prims.cpp +++ b/frontend/lib/resolution/prims.cpp @@ -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: From 83d80ba27bede043ef9bd57b08428296dc344fed Mon Sep 17 00:00:00 2001 From: Lydia Duncan Date: Fri, 22 Nov 2024 08:59:32 -0800 Subject: [PATCH 5/7] Test updates - Update the .bad file to include the better error message. - Add a .future file for the two tests, requesting support for this feature - Add a version of the first test that has an explicit type, to lock in that this particular iteration does not encounter errors in the process of implementing the support I've since lost the future that failed in my earlier test run, but I'm sure it'll turn up again so will fix that in a separate commit ---- Signed-off-by: Lydia Duncan --- test/types/tuple/split-init/initFromAnotherTuple.bad | 10 +++------- .../types/tuple/split-init/initFromAnotherTuple.future | 2 ++ .../tuple/split-init/initFromAnotherTupleTyped.chpl | 6 ++++++ .../tuple/split-init/initFromAnotherTupleTyped.good | 2 ++ test/types/tuple/split-init/initFromFunction.bad | 10 +++------- test/types/tuple/split-init/initFromFunction.future | 2 ++ 6 files changed, 18 insertions(+), 14 deletions(-) create mode 100644 test/types/tuple/split-init/initFromAnotherTuple.future create mode 100644 test/types/tuple/split-init/initFromAnotherTupleTyped.chpl create mode 100644 test/types/tuple/split-init/initFromAnotherTupleTyped.good create mode 100644 test/types/tuple/split-init/initFromFunction.future diff --git a/test/types/tuple/split-init/initFromAnotherTuple.bad b/test/types/tuple/split-init/initFromAnotherTuple.bad index d0f143122d83..cac6154b3f30 100644 --- a/test/types/tuple/split-init/initFromAnotherTuple.bad +++ b/test/types/tuple/split-init/initFromAnotherTuple.bad @@ -1,7 +1,3 @@ -initFromAnotherTuple.chpl:3: internal error: AST-PRI-IVE-0289 chpl version 2.3.0 pre-release (868dbb73be) - -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. - +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 diff --git a/test/types/tuple/split-init/initFromAnotherTuple.future b/test/types/tuple/split-init/initFromAnotherTuple.future new file mode 100644 index 000000000000..461e750996a9 --- /dev/null +++ b/test/types/tuple/split-init/initFromAnotherTuple.future @@ -0,0 +1,2 @@ +feature request: allow split init via tuple syntax when type not declared +#19159 diff --git a/test/types/tuple/split-init/initFromAnotherTupleTyped.chpl b/test/types/tuple/split-init/initFromAnotherTupleTyped.chpl new file mode 100644 index 000000000000..caedf6014536 --- /dev/null +++ b/test/types/tuple/split-init/initFromAnotherTupleTyped.chpl @@ -0,0 +1,6 @@ +var foo = (1,2); +var x,y: int; +(x, y) = (foo(0), foo(1)); + +writeln(x); +writeln(y); diff --git a/test/types/tuple/split-init/initFromAnotherTupleTyped.good b/test/types/tuple/split-init/initFromAnotherTupleTyped.good new file mode 100644 index 000000000000..1191247b6d9a --- /dev/null +++ b/test/types/tuple/split-init/initFromAnotherTupleTyped.good @@ -0,0 +1,2 @@ +1 +2 diff --git a/test/types/tuple/split-init/initFromFunction.bad b/test/types/tuple/split-init/initFromFunction.bad index 337518e3c537..3a4c7830841b 100644 --- a/test/types/tuple/split-init/initFromFunction.bad +++ b/test/types/tuple/split-init/initFromFunction.bad @@ -1,7 +1,3 @@ -initFromFunction.chpl:9: internal error: AST-PRI-IVE-0289 chpl version 2.3.0 pre-release (868dbb73be) - -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. - +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 diff --git a/test/types/tuple/split-init/initFromFunction.future b/test/types/tuple/split-init/initFromFunction.future new file mode 100644 index 000000000000..461e750996a9 --- /dev/null +++ b/test/types/tuple/split-init/initFromFunction.future @@ -0,0 +1,2 @@ +feature request: allow split init via tuple syntax when type not declared +#19159 From dce7c963b55dc66d3f3d5e7641c0a1535e1ca58d Mon Sep 17 00:00:00 2001 From: Lydia Duncan Date: Fri, 22 Nov 2024 09:39:49 -0800 Subject: [PATCH 6/7] Update the .bad file for the older future to use the current message ---- Signed-off-by: Lydia Duncan --- test/types/tuple/split-init/multipleInit.bad | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/test/types/tuple/split-init/multipleInit.bad b/test/types/tuple/split-init/multipleInit.bad index 4f56a94b80d3..1a5f5e9440c6 100644 --- a/test/types/tuple/split-init/multipleInit.bad +++ b/test/types/tuple/split-init/multipleInit.bad @@ -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 From 125a5fcb4ef20eb59663ac686a50222937d48a1a Mon Sep 17 00:00:00 2001 From: Lydia Duncan Date: Fri, 22 Nov 2024 11:08:36 -0800 Subject: [PATCH 7/7] Update the issue the futures for these tests point to I think the Ben McDonald issue is better, and its existence makes it seem like the user issue can be closed, since it tracks what I would want to have tracked by it if I left it open ---- Signed-off-by: Lydia Duncan --- test/types/tuple/split-init/initFromAnotherTuple.future | 2 +- test/types/tuple/split-init/initFromFunction.future | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/test/types/tuple/split-init/initFromAnotherTuple.future b/test/types/tuple/split-init/initFromAnotherTuple.future index 461e750996a9..d81747f7b25b 100644 --- a/test/types/tuple/split-init/initFromAnotherTuple.future +++ b/test/types/tuple/split-init/initFromAnotherTuple.future @@ -1,2 +1,2 @@ feature request: allow split init via tuple syntax when type not declared -#19159 +#16086 diff --git a/test/types/tuple/split-init/initFromFunction.future b/test/types/tuple/split-init/initFromFunction.future index 461e750996a9..d81747f7b25b 100644 --- a/test/types/tuple/split-init/initFromFunction.future +++ b/test/types/tuple/split-init/initFromFunction.future @@ -1,2 +1,2 @@ feature request: allow split init via tuple syntax when type not declared -#19159 +#16086