Fix false positive in _final send checking for generic classes

The finalisers pass conservatively reports FINAL_MAY_SEND when it
encounters a method call on an unknown type. When _final calls a method
on a generic class instantiated with concrete type args (e.g.,
Generic[Prim]), the pass follows into the method body where expressions
still reference the generic type parameter A. Since A's constraint is a
trait, is_known(A) returns false and the pass reports a false positive.

Two changes fix this:

1. Reify the method body with the entity's concrete type arguments
before analyzing it. This replaces type parameters with their concrete
types so is_known sees the actual type. The recursion guard is set on
the original body (not the reified copy) so that recursive lookups
correctly detect cycles.

2. Replace ast_get with lookup_try for method resolution. ast_get only
searches the entity's own symbol table, missing methods inherited
through the provides chain. lookup_try handles the full provides chain.

This expands the range of generic code accepted in _final methods.
Cases where the concrete type doesn't have the called method in its
own scope (e.g., Range calling finite() which exists on the Real trait
but not on USize) still fall back to FINAL_MAY_SEND conservatively.

Closes #4249

Author: SeanTAllen

.github/workflows/ponyc-tier3.yml

@@ -31,7 +31,7 @@ jobs:
       - name: Check for recent commits
         id: check
         run: |
-          if [ "${{ github.event_name }}" = "workflow_dispatch" ]; then
+          if [ "${{ github.event_name }}" != "schedule" ]; then
             echo "has-changes=true" >> "$GITHUB_OUTPUT"
           elif git log --since="7 days ago" --oneline | head -1 | grep -q .; then
             echo "has-changes=true" >> "$GITHUB_OUTPUT"
@@ -69,7 +69,7 @@ jobs:
       - name: Install QEMU
         run: |
           sudo apt-get update -q
-          sudo apt-get install -y -q qemu-utils qemu-system-x86 cloud-image-utils
+          sudo apt-get install -y -q qemu-utils qemu-system-x86 cloud-image-utils expect
           sudo chmod 666 /dev/kvm
       - name: Download FreeBSD image
         run: |
@@ -81,15 +81,16 @@ jobs:
         run: |
           ssh-keygen -t ed25519 -f vm_key -N ""
           PUB_KEY=$(cat vm_key.pub)
+
+          # nuageinit seed: SSH key for freebsd, set root password for su access
           cat > user-data <<USERDATA
           #cloud-config
           ssh_authorized_keys:
             - ${PUB_KEY}
-          packages:
-            - sudo
-            - rsync
-          runcmd:
-            - echo 'freebsd ALL=(ALL) NOPASSWD: ALL' > /usr/local/etc/sudoers.d/freebsd
+          chpasswd:
+            expire: false
+            list:
+              - root:ciroot
           USERDATA
           cat > meta-data <<METADATA
           instance-id: freebsd-ci
@@ -116,19 +117,30 @@ jobs:
               sleep 2
             done
           '
-          echo "SSH available, waiting for cloud-init to finish..."
-          timeout 300 bash -c '
-            while ! ssh -o StrictHostKeyChecking=no -i vm_key -p 2222 freebsd@localhost "test -f /var/lib/cloud/instance/boot-finished" 2>/dev/null; do
-              sleep 5
-            done
-          '
-          echo "VM fully provisioned"
+          echo "SSH available"
       - name: Install build dependencies
         run: |
-          ssh -o StrictHostKeyChecking=no -i vm_key -p 2222 freebsd@localhost <<'EOF'
-          set -e
-          sudo pkg install -y cmake gmake libunwind git python3
-          EOF
+          export PUB_KEY=$(cat vm_key.pub)
+          expect <<'EXPECT'
+          set timeout 600
+          spawn ssh -o StrictHostKeyChecking=no -i vm_key -p 2222 -t freebsd@localhost su -m root
+          expect {
+            "Password:" { send "ciroot\r" }
+            timeout { puts "Timeout waiting for password prompt"; exit 1 }
+          }
+          expect {
+            "#" {}
+            "su: Sorry" { puts "su failed - wrong password or nuageinit didn't set it"; exit 1 }
+            timeout { puts "Timeout waiting for root shell"; exit 1 }
+          }
+          send "pkg install -y cmake gmake libunwind git python3 rsync\r"
+          expect {
+            "#" {}
+            timeout { puts "Timeout during pkg install"; exit 1 }
+          }
+          send "exit\r"
+          expect eof
+          EXPECT
       - name: Copy source to VM
         run: |
           rsync -az -e "ssh -o StrictHostKeyChecking=no -i vm_key -p 2222" \
@@ -153,7 +165,7 @@ jobs:
       - name: Shutdown VM
         if: always()
         run: |
-          ssh -o StrictHostKeyChecking=no -i vm_key -p 2222 freebsd@localhost "sudo shutdown -p now" 2>/dev/null || true
+          pkill -f qemu-system-x86 || true
       - name: Send alert on failure
         if: ${{ failure() }}
         uses: zulip/github-actions-zulip/send-message@e4c8f27c732ba9bd98ac6be0583096dea82feea5

.release-notes/fix-finaliser-generic-false-positive.md

@@ -0,0 +1,5 @@
+## Fix false positive in `_final` send checking for generic classes
+
+The compiler incorrectly rejected `_final` methods that called methods on generic classes instantiated with concrete type arguments. For example, calling a method on a `Generic[Prim]` where `Generic` has a trait-constrained type parameter would produce a spurious "_final cannot create actors or send messages" error, even though the concrete type is known and does not send messages.
+
+The finaliser pass now resolves generic type parameters to their concrete types before analyzing method bodies for potential message sends. This expands the range of generic code accepted in `_final` methods, though some cases involving methods inherited through a provides chain (like `Range`) still produce false positives.

src/libponyc/pass/finalisers.c

@@ -14,7 +14,7 @@ enum
   FINAL_RECURSE = (1 << 2)
 };
 
-static int check_body_send(ast_t* ast, bool in_final);
+static int check_body_send(pass_opt_t* opt, ast_t* ast, bool in_final);
 
 static void show_send(pass_opt_t* opt, ast_t* ast)
 {
@@ -38,62 +38,7 @@ static void show_send(pass_opt_t* opt, ast_t* ast)
   }
 }
 
-static ast_t* receiver_def(ast_t* type)
-{
-  // We must be a known type at this point.
-  switch(ast_id(type))
-  {
-    case TK_ISECTTYPE:
-    {
-      // Find the first concrete type in the intersection.
-      ast_t* child = ast_child(type);
-
-      while(child != NULL)
-      {
-        ast_t* def = receiver_def(child);
-
-        if(def != NULL)
-        {
-          switch(ast_id(def))
-          {
-            case TK_PRIMITIVE:
-            case TK_STRUCT:
-            case TK_CLASS:
-            case TK_ACTOR:
-              return def;
-
-            default: {}
-          }
-        }
-
-        child = ast_sibling(child);
-      }
-
-      break;
-    }
-
-    case TK_NOMINAL:
-      // Return the def.
-      return (ast_t*)ast_data(type);
-
-    case TK_ARROW:
-      // Use the right-hand side.
-      return receiver_def(ast_childidx(type, 1));
-
-    case TK_TYPEPARAMREF:
-    {
-      // Use the constraint.
-      ast_t* def = (ast_t*)ast_data(type);
-      return receiver_def(ast_childidx(def, 1));
-    }
-
-    default: {}
-  }
-
-  return NULL;
-}
-
-static int check_call_send(ast_t* ast, bool in_final)
+static int check_call_send(pass_opt_t* opt, ast_t* ast, bool in_final)
 {
   AST_GET_CHILDREN(ast, lhs, positional, named, question);
   AST_GET_CHILDREN(lhs, receiver, method);
@@ -118,15 +63,60 @@ static int check_call_send(ast_t* ast, bool in_final)
   if(!is_known(type))
     return FINAL_MAY_SEND;
 
-  ast_t* def = receiver_def(type);
-  pony_assert(def != NULL);
-
   const char* method_name = ast_name(method);
-  ast_t* fun = ast_get(def, method_name, NULL);
-  pony_assert(fun != NULL);
+
+  // Use lookup_try to resolve the method, which handles inherited methods
+  // from the provides chain. ast_get only searches the entity's own scope.
+  deferred_reification_t* method_ref = lookup_try(opt, NULL, type,
+    method_name, true);
+
+  if(method_ref == NULL)
+    return FINAL_MAY_SEND;
+
+  ast_t* fun = method_ref->ast;
+  ast_t* def = (ast_t*)ast_data(type);
 
   AST_GET_CHILDREN(fun, cap, id, typeparams, params, result, can_error, body);
-  int r = check_body_send(body, false);
+
+  // If the receiver type has type arguments and the entity has type parameters,
+  // reify the method body so generic type parameters are replaced with their
+  // concrete types. Without this, expressions like `_min.lt(_max)` inside
+  // `Range[USize]` have receiver type `A` (the type parameter), which fails
+  // is_known() and produces a false positive FINAL_MAY_SEND.
+  ast_t* reified_body = NULL;
+  int r;
+
+  if(ast_id(type) == TK_NOMINAL)
+  {
+    ast_t* typeargs = ast_childidx(type, 2);
+    ast_t* entity_typeparams = ast_childidx(def, 1);
+
+    if((ast_id(typeargs) == TK_TYPEARGS) &&
+      (ast_id(entity_typeparams) == TK_TYPEPARAMS))
+    {
+      reified_body = reify(body, entity_typeparams, typeargs, NULL, true);
+    }
+  }
+
+  if(reified_body != NULL)
+  {
+    // Set the recursion guard on the original body so that recursive calls
+    // (which look up the original, not the reified copy) see FINAL_RECURSE.
+    bool already_recursing = ast_checkflag(body, AST_FLAG_RECURSE_1);
+    if(!already_recursing)
+      ast_setflag(body, AST_FLAG_RECURSE_1);
+
+    r = check_body_send(opt, reified_body, false);
+
+    if(!already_recursing)
+      ast_clearflag(body, AST_FLAG_RECURSE_1);
+
+    ast_free_unattached(reified_body);
+  } else {
+    r = check_body_send(opt, body, false);
+  }
+
+  deferred_reify_free(method_ref);
 
   if(r == FINAL_NO_SEND)
   {
@@ -147,27 +137,27 @@ static int check_call_send(ast_t* ast, bool in_final)
   return r;
 }
 
-static int check_expr_send(ast_t* ast, bool in_final)
+static int check_expr_send(pass_opt_t* opt, ast_t* ast, bool in_final)
 {
   int send = FINAL_NO_SEND;
 
   if(ast_id(ast) == TK_CALL)
-    send |= check_call_send(ast, in_final);
+    send |= check_call_send(opt, ast, in_final);
 
   ast_t* child = ast_child(ast);
 
   while(child != NULL)
   {
     if(ast_mightsend(child))
-      send |= check_expr_send(child, in_final);
+      send |= check_expr_send(opt, child, in_final);
 
     child = ast_sibling(child);
   }
 
   return send;
 }
 
-static int check_body_send(ast_t* ast, bool in_final)
+static int check_body_send(pass_opt_t* opt, ast_t* ast, bool in_final)
 {
   if(ast_checkflag(ast, AST_FLAG_RECURSE_1))
     return FINAL_RECURSE;
@@ -180,7 +170,7 @@ static int check_body_send(ast_t* ast, bool in_final)
 
   ast_setflag(ast, AST_FLAG_RECURSE_1);
 
-  int r = check_expr_send(ast, in_final);
+  int r = check_expr_send(opt, ast, in_final);
 
   if(r == FINAL_NO_SEND)
   {
@@ -206,7 +196,7 @@ static bool entity_finaliser(pass_opt_t* opt, ast_t* entity, const char* final)
     return true;
 
   AST_GET_CHILDREN(ast, cap, id, typeparams, params, result, can_error, body);
-  int r = check_body_send(body, true);
+  int r = check_body_send(opt, body, true);
 
   if((r & FINAL_CAN_SEND) != 0 || (r & FINAL_MAY_SEND) != 0)
   {

test/libponyc/finalisers.cc

@@ -111,6 +111,53 @@ TEST_F(FinalisersTest, FinalCannotCallChainedBehaviour)
   TEST_ERRORS_1(src, "_final cannot create actors or send messages");
 }
 
+TEST_F(FinalisersTest, FinalCanCallMethodOnGenericClassWithConcreteTypeArgs)
+{
+  const char* src =
+    "trait Gettable\n"
+    "  fun get(): U32\n"
+    "primitive Prim is Gettable\n"
+    "  fun get(): U32 => 0\n"
+    "class Generic[A: Gettable val]\n"
+    "  let _value: A\n"
+    "  new val create(value: A) =>\n"
+    "    _value = value\n"
+    "  fun get(): U32 =>\n"
+    "    _value.get()\n"
+    "class Foo\n"
+    "  fun _final() =>\n"
+    "    Generic[Prim](Prim).get()";
+
+  TEST_COMPILE(src);
+}
+
+TEST_F(FinalisersTest, FinalCannotCallMethodOnGenericClassThatSends)
+{
+  // The send_msg method sends a message via a behavior call on an actor field.
+  // Even though Generic is instantiated with concrete type args (U32),
+  // the finaliser pass should still detect the send inside send_msg.
+  const char* src =
+    "actor Ping\n"
+    "  new create() => None\n"
+    "  be ping() => None\n"
+    "class Holder\n"
+    "  let _p: Ping\n"
+    "  new create(p: Ping) => _p = p\n"
+    "  fun get(): Ping => _p\n"
+    "class Generic[A]\n"
+    "  let _h: Holder\n"
+    "  new create(h: Holder) => _h = h\n"
+    "  fun send_msg() =>\n"
+    "    _h.get().ping()\n"
+    "class Foo\n"
+    "  let _g: Generic[U32]\n"
+    "  new create(h: Holder) => _g = Generic[U32](h)\n"
+    "  fun _final() =>\n"
+    "    _g.send_msg()";
+
+  TEST_ERRORS_1(src, "_final cannot create actors or send messages");
+}
+
 TEST_F(FinalisersTest, CannotLookupFinal)
 {
   const char* src =