From cccd2fe3a07bf3174a17311d10b2de6abd6d968c Mon Sep 17 00:00:00 2001 From: Danila Fedorin Date: Fri, 24 Jan 2025 10:33:53 -0800 Subject: [PATCH] Respond to reviewer feedback Signed-off-by: Danila Fedorin --- frontend/include/chpl/framework/all-global-strings.h | 1 + frontend/lib/resolution/InitResolver.cpp | 11 +++++++---- frontend/lib/resolution/call-init-deinit.cpp | 3 ++- frontend/lib/resolution/default-functions.cpp | 2 +- frontend/lib/resolution/resolution-queries.cpp | 8 +++++--- 5 files changed, 16 insertions(+), 9 deletions(-) diff --git a/frontend/include/chpl/framework/all-global-strings.h b/frontend/include/chpl/framework/all-global-strings.h index 7746ca3b8296..b21aa8e7fd71 100644 --- a/frontend/include/chpl/framework/all-global-strings.h +++ b/frontend/include/chpl/framework/all-global-strings.h @@ -36,6 +36,7 @@ X(borrow , "borrow") X(borrowed , "borrowed") X(buildTuple , "_build_tuple") X(buildTupleNoref , "_build_tuple_noref") +X(buildTupleAlwaysRef , "_build_tuple_always_allow_ref") X(by , "by") X(bytes , "bytes") X(coforall , "coforall") diff --git a/frontend/lib/resolution/InitResolver.cpp b/frontend/lib/resolution/InitResolver.cpp index d0741de364c8..09025ecd1d36 100644 --- a/frontend/lib/resolution/InitResolver.cpp +++ b/frontend/lib/resolution/InitResolver.cpp @@ -382,9 +382,12 @@ static const ArrayType* arrayTypeFromSubsHelper( if (!instanceBct || !baseArr) return genericArray; if (baseArr->id().symbolPath() == "ChapelDistribution.BaseRectangularArr") { - // Note: the ArrayType predates our '_instance-aware' code (developed - // by Anna and currently at work in Domains). For now, just use the - // "old" style containing a domain type and element type to instantiate the array. + // TODO: the ArrayType predates our '_instance-aware' code (developed + // by Anna and currently at work in Domains). For now, just use the + // "old" style containing a domain type and element type to instantiate + // the array. + // + // Anna is planning on tackling this in future work. auto baseArrRect = baseArr->parentClassType(); CHPL_ASSERT(baseArrRect && baseArrRect->id().symbolPath() == "ChapelDistribution.BaseArrOverRectangularDom"); @@ -400,7 +403,7 @@ static const ArrayType* arrayTypeFromSubsHelper( eltType); } - // If we reach here, we weren't able to resolve the domain type + // If we reach here, we weren't able to resolve the array type return genericArray; } diff --git a/frontend/lib/resolution/call-init-deinit.cpp b/frontend/lib/resolution/call-init-deinit.cpp index e4ddcfaa8f20..74519e275aac 100644 --- a/frontend/lib/resolution/call-init-deinit.cpp +++ b/frontend/lib/resolution/call-init-deinit.cpp @@ -832,7 +832,8 @@ void CallInitDeinit::handleDeclaration(const VarLikeDecl* ast, RV& rv) { frame->addToInitedVars(id); frame->localsAndDefers.push_back(id); } else if (isCatchVariable || isRefLoopIntent) { - // initialized from the throw that activates this Catch + // initialized from the throw that activates this Catch, or implicitly with + // a reference to a variable in outer scope. ID id = ast->id(); frame->addToInitedVars(id); frame->localsAndDefers.push_back(id); diff --git a/frontend/lib/resolution/default-functions.cpp b/frontend/lib/resolution/default-functions.cpp index 59fb91927afa..2a01ee0e7102 100644 --- a/frontend/lib/resolution/default-functions.cpp +++ b/frontend/lib/resolution/default-functions.cpp @@ -1068,7 +1068,7 @@ generateIteratorMethod(Context* context, nullptr)); formalTypes.push_back(QualifiedType(QualifiedType::CONST_REF, it)); - // It's a little scary to compue the ID for the function in this way + // It's a little scary to compute the ID for the function in this way // here because for the FnIterator and PromotionIterator cases, it will // conflict with the underlying function. Maybe that's okay? ID id; diff --git a/frontend/lib/resolution/resolution-queries.cpp b/frontend/lib/resolution/resolution-queries.cpp index ea386f4278bd..6d20ac46da7e 100644 --- a/frontend/lib/resolution/resolution-queries.cpp +++ b/frontend/lib/resolution/resolution-queries.cpp @@ -4118,8 +4118,11 @@ static bool resolveFnCallSpecial(Context* context, // between 'ref' and non-'ref' type formals. Moreover, this function is // filled in by the compiler in production. if (!ci.isMethodCall() && (ci.name() == USTR("_build_tuple") || - ci.name() == USTR("_build_tuple_noref"))) { + ci.name() == USTR("_build_tuple_noref") || + ci.name() == USTR("_build_tuple_always_allow_ref"))) { bool removeRefs = ci.name() == USTR("_build_tuple_noref"); + auto intent = ci.name() == USTR("_build_tuple_always_allow_ref") ? + QualifiedType::CONST_VAR : QualifiedType::TYPE; std::vector components; for (size_t i = 0; i < ci.numActuals(); i++) { auto actual = ci.actual(i).type(); @@ -4131,7 +4134,7 @@ static bool resolveFnCallSpecial(Context* context, } auto resultTuple = TupleType::getQualifiedTuple(context, components); - exprTypeOut = QualifiedType(QualifiedType::TYPE, resultTuple); + exprTypeOut = QualifiedType(intent, resultTuple); return true; } @@ -6769,7 +6772,6 @@ shapeForIteratorQuery(Context* context, break; } } - CHPL_ASSERT(!leaderType.isUnknownOrErroneous()); } CHPL_ASSERT(!leaderType.isUnknownOrErroneous());