Fix a uAST conversion bug that incorrectly converted some array types (…

…chapel-lang#24560)

Resolves chapel-lang#18717.

Fixes a bug which caused an array type preceded by the `borrowed`
keyword to be incorrectly transformed into a loop. This led to internal
errors when the loop was queried for its type information.

At a syntactic level, array types and loops are indistinguishable.
However, in type-only contexts (such as for formal types, or for return
types) we can always convert a "loop" into an array type. This PR
adjusts the uAST converter so that it can handle converting array types
at arbitrary depths. This covers array types that follow the `borrowed`
keyword (which is represented as a "call").

TESTING

- [x] `linux64`, `standard`
- [x] `linux64`, `COMM=gasnet`

Reviewed by @mppf, @vasslitvinov. Thanks!

compiler/passes/convert-uast.cpp

@@ -282,6 +282,7 @@ struct Converter {
   bool inTupleAssign = false;
   bool inImportOrUse = false;
   bool inForwardingDecl = false;
+  bool inTypeExpression = false;
   bool canScopeResolve = false;
   bool trace = false;
   int delegateCounter = 0;
@@ -1869,6 +1870,7 @@ struct Converter {
   // be handled by a separate builder, as those are array types.
   Expr* visit(const uast::BracketLoop* node) {
     if (node->isExpressionLevel()) {
+      if (inTypeExpression) return convertArrayType(node);
       return convertBracketLoopExpr(node);
     } else {
       INT_ASSERT(node->iterand());
@@ -3175,15 +3177,7 @@ struct Converter {
       }
     }
 
-    Expr* retType = nullptr;
-    if (auto retTypeExpr = node->returnType()) {
-      if (auto arrayTypeExpr = retTypeExpr->toBracketLoop()) {
-        retType = convertArrayType(arrayTypeExpr);
-      } else {
-        retType = convertAST(retTypeExpr);
-      }
-    }
-
+    Expr* retType = convertTypeExpressionOrNull(node->returnType());
     Expr* whereClause = convertExprOrNull(node->whereClause());
 
     Expr* lifetimeConstraints = nullptr;
@@ -3343,9 +3337,7 @@ struct Converter {
 
     RetTag retTag = convertRetTag(node->returnIntent());
     auto nodeRetType = node->returnType();
-    Expr* retType = (nodeRetType && nodeRetType->isBracketLoop())
-            ? convertArrayType(nodeRetType->toBracketLoop())
-            : convertExprOrNull(nodeRetType);
+    Expr* retType = convertTypeExpressionOrNull(nodeRetType);
 
     // TODO: I'd like to get rid of these build calls (if Michael
     // has not already gotten rid of them on main), as there's not
@@ -3557,30 +3549,32 @@ struct Converter {
     return INTENT_BLANK;
   }
 
-  Expr* convertTypeExpression(const uast::AstNode* node,
-                              bool isFormalType=false) {
-    if (!node) return nullptr;
-    Expr* ret = nullptr;
+  Expr* convertTypeExpression(const uast::AstNode* node) {
+    INT_ASSERT(node != nullptr);
 
     astlocMarker markAstLoc(node->id());
 
-    if (auto bkt = node->toBracketLoop()) {
-      ret = convertArrayType(bkt, isFormalType);
-    } else {
-      ret = convertAST(node);
-    }
+    bool oldInTypeExpression = inTypeExpression;
+    inTypeExpression = true;
+    Expr* ret = convertAST(node);
+    inTypeExpression = oldInTypeExpression;
 
     INT_ASSERT(ret);
 
     return ret;
   }
 
+  Expr* convertTypeExpressionOrNull(const uast::AstNode* node) {
+    if (!node) return nullptr;
+    return convertTypeExpression(node);
+  }
+
   DefExpr* visit(const uast::Formal* node) {
     IntentTag intentTag = convertFormalIntent(node->intent());
 
     astlocMarker markAstLoc(node->id());
 
-    Expr* typeExpr = convertTypeExpression(node->typeExpression(), true);
+    Expr* typeExpr = convertTypeExpressionOrNull(node->typeExpression());
     Expr* initExpr = convertExprOrNull(node->initExpression());
 
     auto ret =  buildArgDefExpr(intentTag, node->name().c_str(),
@@ -3622,19 +3616,11 @@ struct Converter {
   Expr* visit(const uast::VarArgFormal* node) {
     IntentTag intentTag = convertFormalIntent(node->intent());
 
-    Expr* typeExpr = nullptr;
+    Expr* typeExpr = convertTypeExpressionOrNull(node->typeExpression());
     Expr* initExpr = nullptr;
 
     INT_ASSERT(!node->initExpression());
 
-    if (node->typeExpression()) {
-      if (auto bkt = node->typeExpression()->toBracketLoop()) {
-        typeExpr = convertArrayType(bkt);
-      } else {
-        typeExpr = convertAST(node->typeExpression());
-      }
-    }
-
     Expr* varargsVariable = convertExprOrNull(node->count());
     if (!varargsVariable) {
       varargsVariable = new SymExpr(gUninstantiated);
@@ -3661,7 +3647,7 @@ struct Converter {
     ShadowVarPrefix prefix = convertTaskVarIntent(node);
     // TODO: can we avoid this UnresolvedSymExpr ?
     Expr* nameExp = new UnresolvedSymExpr(node->name().c_str());
-    Expr* type = convertTypeExpression(node->typeExpression());
+    Expr* type = convertTypeExpressionOrNull(node->typeExpression());
     Expr* init = convertExprOrNull(node->initExpression());
 
     auto ret = ShadowVarSymbol::buildForPrefix(prefix, nameExp, type, init);
@@ -3735,8 +3721,7 @@ struct Converter {
     return false;
   }
 
-  CallExpr* convertArrayType(const uast::BracketLoop* node,
-                             bool isFormalType=false) {
+  CallExpr* convertArrayType(const uast::BracketLoop* node) {
     astlocMarker markAstLoc(node->id());
 
     INT_ASSERT(node->isExpressionLevel());
@@ -3774,7 +3759,6 @@ struct Converter {
 
       // If there is a type query, extract it from the domain.
       if (lastTypeQuery) {
-        CHPL_ASSERT(isFormalType);
         CHPL_ASSERT(!domActuals);
         domActuals = convertAST(lastTypeQuery);
       }
@@ -3915,18 +3899,7 @@ struct Converter {
       varSym->cname = convertLinkageNameAstr(node);
     }
 
-    Expr* typeExpr = nullptr;
-
-    // If there is a bracket loop it is almost certainly an array type, so
-    // special case it. Otherwise, just use the generic conversion call.
-    if (const uast::AstNode* te = node->typeExpression()) {
-      if (const uast::BracketLoop* bkt = te->toBracketLoop()) {
-        typeExpr = convertArrayType(bkt);
-      } else {
-        typeExpr = toExpr(convertAST(te));
-      }
-    }
-
+    Expr* typeExpr = convertTypeExpressionOrNull(node->typeExpression());
     Expr* initExpr = nullptr;
 
     if (const uast::AstNode* ie = node->initExpression()) {

compiler/resolution/preFold.cpp

@@ -1430,10 +1430,16 @@ static Expr* preFoldPrimOp(CallExpr* call) {
         msgType = dtBorrowed;
 
       Type* t = e->typeInfo();
-      if (!isClassLikeOrManaged(t))
+      bool emit = true;
+
+      if (auto fn = toFnSymbol(call->parentSymbol)) {
+        emit = !fn->isCompilerGenerated();
+      }
+
+      if (emit && !isClassLikeOrManaged(t))
         USR_FATAL_CONT(call, "cannot make %s into a %s class",
                              toString(t), toString(msgType));
-      if (!isTypeExpr(e))
+      if (emit && !isTypeExpr(e))
         USR_FATAL_CONT(call, "cannot use decorator %s on a value",
                              toString(msgType));
 

test/classes/delete-free/borrowed/borrowed-array.good

@@ -1,3 +1 @@
 borrowed-array.chpl:2: error: cannot make [domain(1,int(64),one)] real(64) into a borrowed class
-borrowed-array.chpl:2: error: assigning to a range with boundKind.both from a range with boundKind.neither without an explicit cast
-note: An additional error is hidden. Use --print-additional-errors to see it.

test/classes/errors/Issue18717.chpl

@@ -0,0 +1,19 @@
+record Chunk {
+  type eltType;
+  var D: domain(1);
+  var data: borrowed [D] eltType;
+
+  proc init(type eltType) {
+    this.eltType = eltType;
+  }
+
+  proc init(array: [?X] ?t) {
+    this.eltType = t;
+    this.D = X;
+    this.data = array;
+  }
+}
+
+proc main() {
+  var x = new Chunk([1, 2, 3]);
+}

test/classes/errors/Issue18717.good

@@ -0,0 +1,3 @@
+Issue18717.chpl:10: In initializer:
+Issue18717.chpl:4: error: cannot make [domain(1,int(64),one)] int(64) into a borrowed class
+  Issue18717.chpl:18: called as Chunk.init(array: [domain(1,int(64),one)] int(64))