dyno: Fix bugs that were breaking chpl-language-server tests

frontend/lib/resolution/resolution-queries.cpp

@@ -539,6 +539,26 @@ static void checkForParenlessMethodFieldRedefinition(Context* context,
   }
 }
 
+// Returns 'true' and emits an error if parent frames are not in the RC.
+static bool errorIfParentFramesNotPresent(ResolutionContext* rc,
+                                          const UntypedFnSignature* usig) {
+  Context* context = rc->context();
+
+  // TODO: More specifically, check if parent frames exist.
+  bool ret = rc->isEmpty();
+
+  // TODO: Becomes a structural issue after we pass in the parent.
+  if (ret) {
+    const ID& id = usig->id();
+    context->error(id, "stack frames for the parent of '%s' are not "
+                       "present, so outer variables in its signature "
+                       "cannot be typed",
+                       usig->name().c_str());
+  }
+
+  return ret;
+}
+
 static const TypedFnSignature*
 typedSignatureInitialImpl(ResolutionContext* rc,
                           const UntypedFnSignature* untypedSig) {
@@ -562,16 +582,26 @@ typedSignatureInitialImpl(ResolutionContext* rc,
         parentSignature = sig;
       }
     } else {
+      // TODO: Have the user pass the parent signature to make this explicit.
       auto parentShape = UntypedFnSignature::get(context, parentFnId);
       parentSignature = typedSignatureInitial(rc, parentShape);
     }
 
     // The parent signature must exist.
     if (!parentSignature) return nullptr;
 
-    // All parents need to be concrete for the signature to be evaluated.
+    // TODO: Change this from a warning to a 'return nullptr' once we pass
+    // in the parent function signature - in that case it is on the caller
+    // to make sure they are concrete.
     for (auto up = parentSignature; up; up = up->parentFn()) {
-      if (up->needsInstantiation()) return nullptr;
+      if (up->needsInstantiation()) {
+        const ID& id = untypedSig->id();
+        context->error(id, "One or more parent functions was inferred "
+                           "to be generic while constructing the "
+                           "initial signature of '%s'",
+                           untypedSig->name().c_str());
+        break;
+      }
     }
   }
 
@@ -581,9 +611,11 @@ typedSignatureInitialImpl(ResolutionContext* rc,
   // visit the formals, but not the return type or body
   for (auto formal : fn->formals()) formal->traverse(visitor);
 
-  // Give up if we could not type outer variables present in the signature.
-  if (parentSignature && rc->isEmpty() && !visitor.outerVariables.isEmpty()) {
-    return nullptr;
+  if (!visitor.outerVariables.isEmpty()) {
+    CHPL_ASSERT(parentSignature);
+
+    // Outer variables can't be typed without stack frames, so give up.
+    if (errorIfParentFramesNotPresent(rc, untypedSig)) return nullptr;
   }
 
   // now, construct a TypedFnSignature from the result
@@ -598,7 +630,7 @@ typedSignatureInitialImpl(ResolutionContext* rc,
   if (auto whereClause = fn->whereClause()) {
     if (needsInstantiation) {
       // Visit the where clause for generic nested functions just to collect
-      // outer variables. TODO: Wrap in speculative block?
+      // outer variables. TODO: Is this OK or could POI muck with this?
       if (parentSignature) whereClause->traverse(visitor);
       whereResult = TypedFnSignature::WHERE_TBD;
     } else {
@@ -607,9 +639,11 @@ typedSignatureInitialImpl(ResolutionContext* rc,
     }
   }
 
-  // Give up if we could not type outer variables used in the where clause.
-  if (parentSignature && rc->isEmpty() && !visitor.outerVariables.isEmpty()) {
-    return nullptr;
+  if (!visitor.outerVariables.isEmpty()) {
+    CHPL_ASSERT(parentSignature);
+
+    // Outer variables can't be typed without stack frames, so give up.
+    if (errorIfParentFramesNotPresent(rc, untypedSig)) return nullptr;
   }
 
   checkForParenlessMethodFieldRedefinition(context, fn, visitor);
@@ -2519,23 +2553,22 @@ resolveFunctionByInfoImpl(ResolutionContext* rc, const TypedFnSignature* sig,
   const AstNode* ast = parsing::idToAst(context, id);
   const Function* fn = ast->toFunction();
   const PoiScope* poiScope = poiInfo.poiScope();
-  bool isInitializer = sig->isInitializer();
   PoiInfo resolvedPoiInfo;
   ResolutionResultByPostorderID rr;
 
-  if (!fn->body() && !isInitializer) {
-    CHPL_ASSERT(false && "Should only be called on functions!");
+  // TODO: Make sure the ID is an extern function specifically.
+  bool canResolveWithoutAst = parsing::idIsExtern(context, sig->id()) ||
+                              sig->isInitializer();
+  if (!fn && !canResolveWithoutAst) {
+    CHPL_ASSERT(false && "Unexpected input to 'resolveFunction'!");
     return nullptr;
   }
 
-  const TypedFnSignature* inputSig = sig;
-  const TypedFnSignature* finalSig = sig;
-
-  auto visitor = isInitializer
-    ? Resolver::createForInitializer(rc, fn, poiScope, inputSig, rr)
-    : Resolver::createForFunction(rc, fn, poiScope, inputSig, rr);
+  auto visitor = sig->isInitializer()
+    ? Resolver::createForInitializer(rc, fn, poiScope, sig, rr)
+    : Resolver::createForFunction(rc, fn, poiScope, sig, rr);
 
-  if (isInitializer) {
+  if (sig->isInitializer()) {
     CHPL_ASSERT(visitor.initResolver.get());
     auto qt = QualifiedType(QualifiedType::VAR, VoidType::get(context));
     visitor.returnType = std::move(qt);
@@ -2549,12 +2582,14 @@ resolveFunctionByInfoImpl(ResolutionContext* rc, const TypedFnSignature* sig,
     return nullptr;
   }
 
+  const TypedFnSignature* finalSig = sig;
+
   // then, compute the return type if it is not an initializer
-  if (!isInitializer) {
+  if (!sig->isInitializer()) {
     computeReturnType(visitor);
 
   // else, potentially write out a new initializer signature
-  } else if (visitor.initResolver) {
+  } else {
     finalSig = visitor.initResolver->finalize();
   }
 
@@ -2566,14 +2601,14 @@ resolveFunctionByInfoImpl(ResolutionContext* rc, const TypedFnSignature* sig,
 
   // check that throws are handled or forwarded
   // TODO: Call for initializers as well, and remove checks in the resolver.
-  if (!isInitializer) checkThrows(rc, rr, fn);
+  if (!sig->isInitializer()) checkThrows(rc, rr, fn);
 
   // TODO: can this be encapsulated in a method?
   resolvedPoiInfo.swap(visitor.poiInfo);
   resolvedPoiInfo.setResolved(true);
   resolvedPoiInfo.setPoiScope(nullptr);
 
-  CHPL_ASSERT(inputSig == finalSig || isInitializer);
+  CHPL_ASSERT(sig == finalSig || sig->isInitializer());
 
   auto ret = toOwned(new ResolvedFunction(finalSig,
                                   fn->returnIntent(),
@@ -2758,7 +2793,6 @@ static const ResolvedFunction*
 helpResolveFunction(ResolutionContext* rc, const TypedFnSignature* sig,
                     const PoiScope* poiScope,
                     bool skipIfRunning) {
-
   // Forget about any inferred signature (to avoid resolving the
   // same function twice when working with inferred 'out' formals)
   sig = sig->inferredFrom();

frontend/lib/resolution/return-type-inference.cpp

@@ -1046,10 +1046,11 @@ static bool helpComputeCompilerGeneratedReturnType(Context* context,
 
 // returns 'true' if it was a case handled here & sets 'result' in that case
 // returns 'false' if it needs to be computed with a ResolvedVisitor traversal
-static bool helpComputeReturnType(Context* context,
+static bool helpComputeReturnType(ResolutionContext* rc,
                                   const TypedFnSignature* sig,
                                   const PoiScope* poiScope,
                                   QualifiedType& result) {
+  Context* context = rc->context();
   const UntypedFnSignature* untyped = sig->untyped();
 
   // TODO: Optimize the order of this case and the isCompilerGenerated case
@@ -1074,8 +1075,7 @@ static bool helpComputeReturnType(Context* context,
 
       // resolve the return type
       ResolutionResultByPostorderID resolutionById;
-      ResolutionContext rc(context);
-      auto visitor = Resolver::createForFunction(&rc, fn, poiScope, sig,
+      auto visitor = Resolver::createForFunction(rc, fn, poiScope, sig,
                                                  resolutionById);
       retType->traverse(visitor);
       result = resolutionById.byAst(retType).type();
@@ -1146,7 +1146,7 @@ static const QualifiedType& returnTypeQuery(ResolutionContext* rc,
   const UntypedFnSignature* untyped = sig->untyped();
   QualifiedType result;
 
-  bool computed = helpComputeReturnType(context, sig, poiScope, result);
+  bool computed = helpComputeReturnType(rc, sig, poiScope, result);
   if (!computed) {
     const AstNode* ast = parsing::idToAst(context, untyped->id());
     const Function* fn = ast->toFunction();
@@ -1232,7 +1232,7 @@ const TypedFnSignature* inferOutFormals(ResolutionContext* rc,
 
 void computeReturnType(Resolver& resolver) {
   QualifiedType returnType;
-  bool computed = helpComputeReturnType(resolver.context,
+  bool computed = helpComputeReturnType(resolver.rc,
                                         resolver.typedSignature,
                                         resolver.poiScope,
                                         returnType);

frontend/test/resolution/testResolve.cpp

@@ -1688,6 +1688,26 @@ static void test26() {
   }
 }
 
+// Make sure that 'extern' functions still have 'ResolvedFunction' entries.
+static void test27() {
+  Context ctx;
+  Context* context = &ctx;
+  ErrorGuard guard(context);
+
+  // 'this' qualified, type field
+  std::string prog =
+    R"""(
+      extern proc foo(): int;
+    )""";
+
+  auto m = parseModule(context, prog);
+  auto f = m->stmt(0)->toFunction();
+  assert(f && f->linkage() == Decl::EXTERN);
+  auto rf = resolveConcreteFunction(context, f->id());
+  assert(rf->returnType().type()->isIntType());
+  assert(guard.realizeErrors() == 0);
+}
+
 int main() {
   test1();
   test2();
@@ -1715,6 +1735,7 @@ int main() {
   test24();
   test25();
   test26();
+  test27();
 
   return 0;
 }