Dyno: Fix erroneous ambiguity error for nested functions (chapel-lang…

…#26058)

Logic in ``resolveIdentifier`` was looking for ambiguity errors between
parenless methods and locally-declared variables, but was instead
issuing errors for paren-ful methods and nested functions as well.

This commit changes resolveIdentifier to only run these errors if a
parenless method is found and if there are redeclarations other than the
paren-less method.

``test15`` in ``testMethodCalls`` is temporarily retired as it is not
actually considered to be an ambiguity in production. This instead
appears to be a bug in which we incorrectly prefer the method over the
nested function.

[reviewed-by @riftEmber]

compiler/passes/convert-uast.cpp

@@ -5182,6 +5182,8 @@ void Converter::postConvertApplyFixups() {
 
     INT_ASSERT(isTemporaryConversionSymbol(se->symbol()));
 
+    astlocMarker markAstLoc(target->id());
+
     Symbol* sym = findConvertedFn(target, /* neverTrace */ true);
     if (!isFnSymbol(sym)) {
       INT_FATAL(se, "could not find target function for call fixup %s",

frontend/lib/resolution/Resolver.cpp

@@ -3335,20 +3335,33 @@ void Resolver::resolveIdentifier(const Identifier* ident) {
       auto inScope = scopeStack.back();
       auto inScopes = CallScopeInfo::forNormalCall(inScope, poiScope);
       auto c = resolveGeneratedCall(context, ident, ci, inScopes);
-      // Ensure we error out for redeclarations within the method itself,
-      // which resolution with an implicit 'this' currently does not catch.
       MatchingIdsWithName redeclarations;
       inScope->lookupInScope(ident->name(), redeclarations, IdAndFlags::Flags(),
                              IdAndFlags::FlagSet());
-      if (!redeclarations.isEmpty()) {
-        LookupConfig config = IDENTIFIER_LOOKUP_CONFIG;
-        if (!resolvingCalledIdent) config |= LOOKUP_INNERMOST;
-        issueAmbiguityErrorIfNeeded(ident, inScope, config);
-      } else {
-        // Save result if successful
-        if (handleResolvedCallWithoutError(result, ident, ci, c) &&
-            emitLookupErrors) {
-          issueErrorForFailedCallResolution(ident, ci, c);
+      if (c.mostSpecific().numBest() == 1) {
+        // A local variable would be ambiguous with a paren-less method, so
+        // let's check for redeclarations within the current method.
+        if (!redeclarations.isEmpty()) {
+          auto only = c.mostSpecific().only();
+          bool otherThanParenless = false;
+          for (auto& elt : redeclarations) {
+            if (only.fn()->id() != elt) {
+              otherThanParenless = true;
+              break;
+            }
+          }
+
+          if (otherThanParenless) {
+            LookupConfig config = IDENTIFIER_LOOKUP_CONFIG;
+            if (!resolvingCalledIdent) config |= LOOKUP_INNERMOST;
+            issueAmbiguityErrorIfNeeded(ident, inScope, config);
+          }
+        } else {
+          // Save result if successful
+          if (handleResolvedCallWithoutError(result, ident, ci, c) &&
+              emitLookupErrors) {
+            issueErrorForFailedCallResolution(ident, ci, c);
+          }
         }
       }
     } else {

frontend/test/resolution/testMethodCalls.cpp

@@ -739,34 +739,6 @@ static void test14b() {
   assert(guard.realizeErrors() == 2);
 }
 
-static void test15() {
-  // Test ambiguity emitted between nested function and method.
-  Context ctx;
-  Context* context = &ctx;
-  ErrorGuard guard(context);
-
-  std::string program = R"""(
-      class Foo {
-        proc init() {}
-
-        proc asdf() {
-          return 1;
-        }
-
-        proc doSomething() {
-          proc asdf() do return 2;
-          return asdf();
-        }
-      }
-
-      var f = new Foo();
-      var x = f.doSomething();
-      )""";
-
-  auto vars = resolveTypesOfVariables(context, program, { "x" });
-  assert(guard.realizeErrors() == 1);
-}
-
 static void test16() {
   // Test resolving 'this' call on variable that shadows field
 
@@ -897,7 +869,6 @@ int main() {
   test13();
   test14();
   test14b();
-  test15();
   test16();
   test17();
 

frontend/test/resolution/testNestedFunctions.cpp

@@ -454,6 +454,62 @@ static void test10(void) {
 }
 */
 
+static void test11() {
+  Context context;
+  Context* ctx = turnOnWarnUnstable(&context);
+  ErrorGuard guard(ctx);
+
+  std::string program =
+    R""""(
+    record R { }
+
+    proc R.test() {
+      proc foobar(arg: int) { return arg; }
+      proc foobar(arg: real) { return arg; }
+      proc foobar(arg: string) { return arg; }
+
+      return foobar("test");
+    }
+
+    var r : R;
+    var x = r.test();
+    )"""";
+
+  auto vars = resolveTypesOfVariables(ctx, program, {"x"});
+  auto x = vars["x"];
+  assert(x.type()->isStringType());
+}
+
+// TODO: incorrectly chooses method over nested function
+//static void test12() {
+//  // Test ambiguity emitted between nested function and method.
+//  Context ctx;
+//  Context* context = &ctx;
+//  ErrorGuard guard(context);
+//
+//  std::string program = R"""(
+//      class Foo {
+//        proc init() {}
+//
+//        proc asdf() {
+//          return "test";
+//        }
+//
+//        proc doSomething() {
+//          proc asdf() do return 2;
+//          return asdf();
+//        }
+//      }
+//
+//      var f = new Foo();
+//      var x = f.doSomething();
+//      )""";
+//
+//  auto vars = resolveTypesOfVariables(context, program, { "x" });
+//  auto x = vars["x"];
+//  assert(x.type()->isIntType());
+//}
+
 int main() {
   test0();
   test1();
@@ -466,5 +522,6 @@ int main() {
   // test8();
   test9();
   // test10();
+  test11();
   return 0;
 }