Respond to review feedback

chapel-lang · Sep 8, 2020 · 0d30c64 · 0d30c64
1 parent 32d4bfd
commit 0d30c64
Show file tree

Hide file tree

Showing 3 changed files with 13 additions and 8 deletions.
diff --git a/compiler/resolution/ResolutionCandidate.cpp b/compiler/resolution/ResolutionCandidate.cpp
@@ -466,8 +466,8 @@ void clearCoercibleCache() {
   actualFormalCoercible.clear();
 }
 
-//
-// in a way that is appropriate for uses when resolving arguments
+// Uses formalSym and actualSym to compute allowCoercion, implicitBang, and
+// inOutCopy in a way that is appropriate for uses when resolving arguments
 static
 Type* getInstantiationType(Symbol* actual, ArgSymbol* formal, Expr* ctx) {
   bool allowCoercions = shouldAllowCoercions(actual, formal);

diff --git a/compiler/resolution/callDestructors.cpp b/compiler/resolution/callDestructors.cpp
@@ -763,14 +763,18 @@ bool isTemporaryFromNoCopyReturn(Symbol* fromSym) {
       if (isInitOrReturn(call, lhsSe, initOrCtor)) {
         if (lhsSe == se) {
           if (initOrCtor != NULL) {
-            if (FnSymbol* calledFn = initOrCtor->resolvedOrVirtualFunction())
-              if (calledFn->hasFlag(FLAG_NO_COPY_RETURN))
+            if (FnSymbol* calledFn = initOrCtor->resolvedOrVirtualFunction()) {
+              if (calledFn->hasFlag(FLAG_NO_COPY_RETURN)) {
                 anyNoCopyReturn = true;
+              }
+            }
           } else {
             // Track moving from another temp.
-            if (SymExpr* rhsSe = toSymExpr(call->get(2)))
-              if (isTemporaryFromNoCopyReturn(rhsSe->symbol()))
+            if (SymExpr* rhsSe = toSymExpr(call->get(2))) {
+              if (isTemporaryFromNoCopyReturn(rhsSe->symbol())) {
                 anyNoCopyReturn = true;
+              }
+            }
           }
         }
       }
@@ -795,6 +799,7 @@ bool doesCopyInitializationRequireCopy(Expr* initFrom) {
 
     // Is it the result of a call returning by value?
     SymExpr* fromSe = toSymExpr(initFrom);
+    INT_ASSERT(fromSe != NULL); // assuming normalized AST
     if (isCallExprTemporary(fromSe->symbol())) {
       // check for the sub-expression being a function marked with
       //  pragma "no copy return"

diff --git a/compiler/resolution/resolveFunction.cpp b/compiler/resolution/resolveFunction.cpp
@@ -769,13 +769,13 @@ static void insertUnrefForArrayOrTupleReturn(FnSymbol* fn) {
                            isTupleContainingAnyReferences(rhsType);
 
         if ((handleArray || handleDomain || handleTuple) &&
-            !isTypeExpr(call->get(2))) {
+            !isTypeExpr(fromExpr)) {
 
           FnSymbol* initCopyFn = getInitCopyDuringResolution(rhsType);
           INT_ASSERT(initCopyFn);
 
           SET_LINENO(call);
-          Expr*      rhs       = call->get(2)->remove();
+          Expr*      rhs       = fromExpr->remove();
           VarSymbol* tmp       = newTemp("copy_ret_tmp", rhsType);
           CallExpr*  initTmp   = new CallExpr(PRIM_MOVE,     tmp, rhs);
           Symbol* definedConst = gFalse;