Dyno: resolution fixes variety pack (#26623)

This PR goes through the resolution meta-issue and fixes a number of
relatively simple issues encountered in the course of resolving the
specs.

Closes Cray/chapel-private#7036 (querying
vararg size in `where` clause)
Closes Cray/chapel-private#7034 ("passing
nested enums to generic functions", but actually resolving nested enums
at all)
Closes Cray/chapel-private#7033 (`range(?)`
formals becoming concrete; more broadly, `R(?)` formals becoming
concrete for generic-with-defaults `R`)
Closes Cray/chapel-private#7030 (resolving
`new` on type aliases to instantiated types).
Closes Cray/chapel-private#6927 (assigning
`nil` to `ddata`, but broadly fixing candidate selection w/ generic
formals and subtype conversions)

Reviewed by @riftEmber -- thanks!

## Testing
- [x] dyno tests
- [x] paratest

frontend/include/chpl/resolution/can-pass.h

@@ -96,6 +96,8 @@ class CanPassResult {
   static bool isTypeGeneric(Context* context, const types::QualifiedType& qt);
   static bool isTypeGeneric(Context* context, const types::Type* t);
 
+  static CanPassResult ensureSubtypeConversionInstantiates(CanPassResult r);
+
   static bool
   canConvertNumeric(Context* context,
                     const types::Type* actualT,

frontend/include/chpl/resolution/resolution-error-classes-list.h

@@ -75,6 +75,7 @@ ERROR_CLASS(IteratorsInOtherScopes, const uast::AstNode*, const resolution::Type
 ERROR_CLASS(MemManagementNonClass, const uast::New*, const types::Type*)
 ERROR_CLASS(MissingInclude, const uast::Include*, std::string)
 ERROR_CLASS(MissingFormalInstantiation, const uast::AstNode*, std::vector<std::tuple<const uast::Decl*, types::QualifiedType>>)
+ERROR_CLASS(MismatchedInitializerResult, const uast::AstNode*, const types::CompositeType*, const types::CompositeType*, std::vector<std::tuple<ID, chpl::UniqueString, types::QualifiedType, types::QualifiedType>>)
 ERROR_CLASS(ModuleAsVariable, const uast::AstNode*, const uast::AstNode*, const uast::Module*)
 ERROR_CLASS(MultipleEnumElems, const uast::AstNode*, chpl::UniqueString, const uast::Enum*, std::vector<ID>)
 ERROR_CLASS(MultipleInheritance, const uast::Class*, const uast::AstNode*, const uast::AstNode*)

frontend/lib/resolution/InitResolver.cpp

@@ -454,6 +454,12 @@ static const Type* ctFromSubs(Context* context,
 const Type* InitResolver::computeReceiverTypeConsideringState(void) {
   auto ctInitial = initialRecvType_->getCompositeType();
 
+  // for the purposes of determing if subs are needed, we want to inspect the
+  // base type.
+  if (auto ctInitialBase = ctInitial->instantiatedFromCompositeType()) {
+    ctInitial = ctInitialBase;
+  }
+
   // The non-default fields are used to determine if we need to create
   // substitutions. I.e., if a field is concrete even if we ignore defaults,
   // no reason to add a substitution.
@@ -791,8 +797,8 @@ bool InitResolver::applyResolvedInitCallToState(const FnCall* node,
   }
 
   auto initialCompType = initialRecvType_->getCompositeType();
-  if (initialCompType->instantiatedFromCompositeType()) {
-    initialCompType = initialCompType->instantiatedFromCompositeType();
+  if (auto initialCompTypeBase = initialCompType->instantiatedFromCompositeType()) {
+    initialCompType = initialCompTypeBase;
   }
 
   CHPL_ASSERT(receiverCompType == initialCompType);

frontend/lib/resolution/Resolver.cpp

@@ -1255,16 +1255,23 @@ void Resolver::resolveTypeQueries(const AstNode* formalTypeExpr,
   }
 }
 
+void Resolver::resolveVarArgSizeQuery(const uast::VarArgFormal* varArgFormal,
+                                      int numVarArgs) {
+  if (auto countQuery = varArgFormal->count()) {
+    if (countQuery->isTypeQuery()) {
+      ResolvedExpression& result = byPostorder.byAst(countQuery);
+      result.setType(QualifiedType::makeParamInt(context, numVarArgs));
+    }
+  }
+}
+
 void Resolver::resolveTypeQueriesFromFormalType(const VarLikeDecl* formal,
                                                 QualifiedType formalType) {
   if (auto varargs = formal->toVarArgFormal()) {
     const TupleType* tuple = formalType.type()->toTupleType();
 
     // args...?n
-    if (auto countQuery = varargs->count()) {
-      ResolvedExpression& result = byPostorder.byAst(countQuery);
-      result.setType(QualifiedType::makeParamInt(context, tuple->numElements()));
-    }
+    resolveVarArgSizeQuery(varargs, tuple->numElements());
 
     if (auto typeExpr = formal->typeExpression()) {
       QualifiedType useType = tuple->elementType(0);
@@ -1831,7 +1838,12 @@ void Resolver::resolveNamedDecl(const NamedDecl* decl, const Type* useType) {
         if (inferFromInit == false && !isTypeOrParam)  {
           // check also for a generic type as the type expression
           auto g = getTypeGenericity(context, typeExprT);
-          if (g != Type::GENERIC) {
+
+          // "generic with defaults" at this point means "generic" because
+          // getTypeGenericity considers the type expression's substitutions,
+          // and existing logic would have inserted the defaults if possible.
+          // Thus, this expression is explicitly generic.
+          if (g != Type::GENERIC && g != Type::GENERIC_WITH_DEFAULTS) {
             inferFromInit = true;
           }
         }
@@ -2470,6 +2482,60 @@ void Resolver::resolveTupleDecl(const TupleDecl* td,
   resolveTupleUnpackDecl(td, useT);
 }
 
+static bool addExistingSubstitutionsAsActuals(Context* context,
+                                              const Type* type,
+                                              std::vector<CallInfoActual>& outActuals,
+                                              std::vector<const AstNode*>& outActualAsts) {
+  bool addedSubs = false;
+  while (auto ct = type->getCompositeType()) {
+    if (!ct->instantiatedFromCompositeType()) break;
+
+    for (auto& [id, qt] : ct->substitutions()) {
+      auto fieldName = parsing::fieldIdToName(context, id);
+      addedSubs = true;
+      outActuals.emplace_back(qt, fieldName);
+      outActualAsts.push_back(nullptr);
+    }
+
+    if (auto clt = ct->toBasicClassType()) {
+      type = clt->parentClassType();
+    } else {
+      break;
+    }
+  }
+
+  return addedSubs;
+}
+
+static void findMismatchedInstantiations(Context* context,
+                                         const CompositeType* originalCT,
+                                         const CompositeType* finalCT,
+                                         std::vector<std::tuple<ID, UniqueString, QualifiedType, QualifiedType>>& out) {
+  while (originalCT) {
+    CHPL_ASSERT(finalCT);
+    if (!originalCT->instantiatedFromCompositeType()) break;
+
+    for (auto& [id, qt] : originalCT->substitutions()) {
+      auto finalSub = finalCT->substitutions().find(id);
+      if (finalSub == finalCT->substitutions().end()) {
+        out.emplace_back(id, parsing::fieldIdToName(context, id), qt, QualifiedType());
+      } else {
+        auto& finalQt = finalSub->second;
+        if (qt != finalQt) {
+          out.emplace_back(id, parsing::fieldIdToName(context, id), qt, finalQt);
+        }
+      }
+    }
+
+    if (auto clt = originalCT->toBasicClassType()) {
+      originalCT = clt->parentClassType();
+      finalCT = finalCT->toBasicClassType()->parentClassType();
+    } else {
+      break;
+    }
+  }
+}
+
 bool Resolver::resolveSpecialNewCall(const Call* call) {
   if (!call->calledExpression() ||
       !call->calledExpression()->isNew()) {
@@ -2522,6 +2588,14 @@ bool Resolver::resolveSpecialNewCall(const Call* call) {
   auto receiverInfo = CallInfoActual(calledType, USTR("this"));
   actuals.push_back(std::move(receiverInfo));
   actualAsts.push_back(newExpr->typeExpression());
+
+  // if the type has existing substitutions, we feed them as named actuals
+  // to the constructor. However, the constructor can do something entirely
+  // different, and override the subs we passed in. This is an error, which
+  // we should check for if we added named actuals.
+  bool validateFinalType =
+    addExistingSubstitutionsAsActuals(context, qtNewExpr.type(), actuals, actualAsts);
+
   // Remaining actuals.
   prepareCallInfoActuals(call, actuals, questionArg, &actualAsts);
   CHPL_ASSERT(!questionArg);
@@ -2566,6 +2640,19 @@ bool Resolver::resolveSpecialNewCall(const Call* call) {
 
     auto qt = QualifiedType(QualifiedType::VAR, type);
     re.setType(qt);
+
+    if (validateFinalType) {
+      auto originalCT = initReceiverType->getCompositeType();
+      auto finalCT = type->getCompositeType();
+      CHPL_ASSERT(originalCT);
+      CHPL_ASSERT(finalCT);
+
+      if (!finalCT->isInstantiationOf(context, originalCT)) {
+        std::vector<std::tuple<ID, UniqueString, QualifiedType, QualifiedType>> mismatches;
+        findMismatchedInstantiations(context, originalCT, finalCT, mismatches);
+        CHPL_REPORT(context, MismatchedInitializerResult, call, originalCT, finalCT, std::move(mismatches));
+      }
+    }
   }
 
   return true;
@@ -3004,6 +3091,9 @@ QualifiedType Resolver::typeForId(const ID& id, bool localGenericToUnknown) {
     return QualifiedType(QualifiedType::TYPE, t);
   } else if (asttags::isModule(tag)) {
     return QualifiedType(QualifiedType::MODULE, nullptr);
+  } else if (asttags::isEnum(tag)) {
+    const Type* t = initialTypeForTypeDecl(context, id);
+    return QualifiedType(QualifiedType::TYPE, t);
   } else if (asttags::isInterface(tag)) {
     const Type* t = initialTypeForInterface(context, id);
     return QualifiedType(QualifiedType::TYPE, t);

frontend/lib/resolution/Resolver.h

@@ -394,6 +394,9 @@ struct Resolver {
                           bool isNonStarVarArg = false,
                           bool isTopLevel = true);
 
+  void resolveVarArgSizeQuery(const uast::VarArgFormal* varArgFormal,
+                              int numVarArgs);
+
   /* When resolving a function with a TypeQuery, we need to
      resolve the type that is queried, since it can be used
      on its own later.

frontend/lib/resolution/can-pass.cpp

@@ -519,8 +519,12 @@ CanPassResult CanPassResult::canPassSubtypeNonBorrowing(Context* context,
                                             const Type* formalT) {
   // nil -> pointers
   if (actualT->isNilType() && formalT->isNilablePtrType() &&
-      !formalT->isCStringType())
-    return convert(SUBTYPE);
+      !formalT->isCStringType()) {
+    return CanPassResult(/* no fail reason */ {},
+                         /* instantiates */ false,
+                         /*promotes*/ false,
+                         /*conversion*/ SUBTYPE);
+  }
 
   // class types
   if (auto actualCt = actualT->toClassType()) {
@@ -887,6 +891,21 @@ shouldConvertClassTypeIntoManagerRecord(Context* context,
   return empty;
 }
 
+CanPassResult CanPassResult::ensureSubtypeConversionInstantiates(CanPassResult got) {
+  if (got.instantiates()) {
+    return got;
+  }
+
+  // The type passed, but didn't actually instantiate. This is odd
+  // since we are trying to instantiate a generic formal. This suggests
+  // the actual is generic, too, which typically doesn't make sense.
+  // Explicitly set the fail reason but keep the rest of the properties
+  // intact, so that that the caller can dismiss this error if
+  // generic actuals are allowed.
+  got.failReason_ = FAIL_DID_NOT_INSTANTIATE;
+  return got;
+}
+
 CanPassResult CanPassResult::canInstantiate(Context* context,
                                             const QualifiedType& actualQT,
                                             const QualifiedType& formalQT) {
@@ -908,15 +927,11 @@ CanPassResult CanPassResult::canInstantiate(Context* context,
     return instantiate();
   }
 
-  // TODO: Should we move this to the section below and have it call canPassSubtypeOrBorrowing?
-  // TODO: There may be cases for nilType that are not covered
   // this is to allow instantiating 'class?' type with nil
-  if (auto cls = formalT->toClassType()) {
-    if (auto mt = cls->manageableType()) {
-      if (mt->isAnyClassType()) {
-        if (cls->decorator().isNilable() && actualT->isNilType())
-          return instantiate();
-      }
+  if (actualT->isNilType()) {
+    auto got = canPassSubtypeOrBorrowing(context, actualT, formalT);
+    if (got.passes()) {
+      return ensureSubtypeConversionInstantiates(got);
     }
   }
 
@@ -935,18 +950,7 @@ CanPassResult CanPassResult::canInstantiate(Context* context,
     if (auto formalCt = formalT->toClassType()) {
       CanPassResult got = canPassSubtypeOrBorrowing(context, actualCt, formalCt);
       if (got.passes()) {
-        if (got.instantiates()) {
-          return got;
-        }
-
-        // The type passed, but didn't actually instantiate. This is odd
-        // since we are trying to instantiate a generic formal. This suggests
-        // the actual is generic, too, which typically doesn't make sense.
-        // Explicitly set the fail reason but keep the rest of the properties
-        // intact, so that that the caller can dismiss this error if
-        // generic actuals are allowed.
-        got.failReason_ = FAIL_DID_NOT_INSTANTIATE;
-        return got;
+        return ensureSubtypeConversionInstantiates(got);
       }
     }
   } else if (auto actualCt = actualT->toCompositeType()) {

frontend/lib/resolution/resolution-error-classes-list.cpp

@@ -1213,6 +1213,32 @@ void ErrorMissingFormalInstantiation::write(ErrorWriterBase& wr) const {
   }
 }
 
+void ErrorMismatchedInitializerResult::write(ErrorWriterBase& wr) const {
+  auto node = std::get<0>(info_);
+  auto initialCT = std::get<1>(info_);
+  auto finalCT = std::get<2>(info_);
+  auto& mismatches = std::get<3>(info_);
+
+  wr.heading(kind_, type_, node, "final result of initialization does not match the initial type.");
+  wr.message("In the following initialization: ");
+  wr.code(justOneLine(node), { node });
+  wr.message("The initializer was invoked with an instantiated type '", initialCT, "'.");
+  wr.message("However, the initializer resulted in a value of type '", finalCT, "'.");
+
+  for (auto& mismatch : mismatches) {
+    auto& id = std::get<0>(mismatch);
+    auto fieldName = std::get<1>(mismatch);
+    auto& expectedType = std::get<2>(mismatch);
+    auto& actualType = std::get<3>(mismatch);
+
+    wr.message("");
+    wr.note(id, "field '", fieldName, "' started with type '", expectedType.type(),
+            "', but had incompatible type '", actualType.type(), "' "
+            "after initialization.");
+    wr.codeForDef(id);
+  }
+}
+
 void ErrorModuleAsVariable::write(ErrorWriterBase& wr) const {
   auto node = std::get<0>(info_);
   auto parent = std::get<1>(info_);

frontend/lib/resolution/resolution-queries.cpp

@@ -2417,6 +2417,7 @@ ApplicabilityResult instantiateSignature(ResolutionContext* rc,
       QualifiedType vat = QualifiedType(formal->storageKind(), t);
       substitutions.insert({formal->id(), vat});
       r.byAst(formal).setType(vat);
+      visitor.resolveVarArgSizeQuery(formal, t->numElements());
 
       // note that a substitution was used here
       if ((size_t) varArgIdx >= formalsInstantiated.size()) {
@@ -3288,7 +3289,15 @@ isInitialTypedSignatureApplicable(Context* context,
       } else {
         got = canPassFn(context, actualType, formalType);
       }
-      if (!got.passes()) {
+      if (got.passes()) {
+        // fine, continue to next entry
+      } else if (got.reason() == FAIL_DID_NOT_INSTANTIATE) {
+        // A conversion is possible, but the formal type is generic and
+        // the actual doesn't have enough info to instantiate. However,
+        // this is only the "initial" check -- previous formals might contribute
+        // information that makes the formal concrete. Allow this for now.
+      } else {
+        CHPL_ASSERT(!got.passes());
         return ApplicabilityResult::failure(tfs, got.reason(), entry.formalIdx());
       }
     }

frontend/lib/resolution/signature-checks.cpp

@@ -61,7 +61,8 @@ static const bool& checkSignatureQuery(Context* context,
     auto thisIntent = sig->formalType(0).kind();
     auto rhsIntent = sig->formalType(1).kind();
     // check the intent of the 'this' argument
-    if (!(isGenericQualifier(thisIntent) || isRefQualifier(thisIntent))) {
+    if (!(isGenericQualifier(thisIntent) || isRefQualifier(thisIntent) ||
+          (isInQualifier(thisIntent) && sig->formalType(0).type()->isClassType()))) {
       context->error(errId, "Bad 'this' intent for init=");
     }
     bool rhsIntentGenericOrRef = isGenericQualifier(rhsIntent) ||

frontend/test/resolution/testEnums.cpp

@@ -719,6 +719,28 @@ static void test21() {
   check(vars.at("z"), "blue");
 }
 
+static void test22() {
+  auto context = buildStdContext();
+  QualifiedType qt =  resolveTypeOfXInit(context,
+                         R""""(
+                         proc id(param x) param do return x;
+                         proc foo() param {
+                           enum color {
+                             red, green, blue
+                           }
+
+                           return id(color.red);
+                         }
+
+                         var x = foo();
+                         )"""");
+  assert(qt.kind() == QualifiedType::PARAM);
+  assert(qt.type() && qt.type()->isEnumType());
+  assert(qt.param() && qt.param()->isEnumParam());
+
+  ensureParamEnumStr(qt, "red");
+}
+
 int main() {
   test1();
   test2();
@@ -741,6 +763,7 @@ int main() {
   test19();
   test20();
   test21();
+  test22();
 
   return 0;
 }