Reject identity comparison between distinct concrete types

.release-notes/error-on-disjoint-concrete-is-comparison.md

@@ -0,0 +1,18 @@
+## Reject identity comparison between distinct concrete types
+
+The compiler now rejects `is` and `isnt` comparisons whose operand types are two distinct concrete entity types (any pair drawn from `class`, `actor`, `primitive`, and `struct`). Such comparisons can never be true, so they are almost always a bug; flagging them at compile time surfaces the mistake immediately instead of producing a constant `false` at runtime.
+
+```pony
+class C
+class D
+
+actor Main
+  new create(env: Env) =>
+    let c: C = C
+    let d: D = D
+    if c is d then None end  // now a compile-time error
+```
+
+The rule fires only when both operands resolve to a single concrete nominal type. Comparisons involving traits, interfaces, unions, intersections, tuples, or type parameters continue to compile, even when one side is concrete. Two reifications of the same generic class (for example `Array[U64]` and `Array[U32]`) share a single root definition and are also not flagged. Two `object end` literals at different source locations synthesize distinct anonymous classes and are flagged.
+
+If you hit the new error, the comparison was unreachable. Switch to structural equality (`==`) or remove the comparison.

src/libponyc/pass/verify.c

@@ -3,6 +3,7 @@
 #include "../type/cap.h"
 #include "../type/compattype.h"
 #include "../type/lookup.h"
+#include "../type/typealias.h"
 #include "../verify/call.h"
 #include "../verify/control.h"
 #include "../verify/fun.h"
@@ -606,12 +607,131 @@ static bool verify_is_comparand(pass_opt_t* opt, ast_t* ast)
   }
 }
 
+// If `type` resolves to a singular concrete nominal entity (TK_CLASS, TK_ACTOR,
+// TK_PRIMITIVE, or TK_STRUCT), returns the entity definition pointer. Returns
+// NULL for anything else (unions, intersections, tuples, type parameters,
+// this-types, trait/interface nominals, or unresolvable aliases).
+//
+// Unwraps TK_TYPEALIASREF and TK_ARROW transitively. If an unfolded alias is
+// allocated, it is written to *to_free so the caller can release it with
+// ast_free_unattached after using the returned pointer.
+static ast_t* concrete_nominal_def(ast_t* type, ast_t** to_free)
+{
+  *to_free = NULL;
+
+  while(type != NULL)
+  {
+    switch(ast_id(type))
+    {
+      case TK_TYPEALIASREF:
+      {
+        ast_t* unfolded = typealias_unfold(type);
+        if(unfolded == NULL)
+          return NULL;
+        if(*to_free != NULL)
+          ast_free_unattached(*to_free);
+        *to_free = unfolded;
+        type = unfolded;
+        continue;
+      }
+
+      case TK_ARROW:
+        type = ast_childidx(type, 1);
+        continue;
+
+      case TK_NOMINAL:
+      {
+        ast_t* def = (ast_t*)ast_data(type);
+        if(def == NULL)
+          return NULL;
+
+        switch(ast_id(def))
+        {
+          case TK_CLASS:
+          case TK_ACTOR:
+          case TK_PRIMITIVE:
+          case TK_STRUCT:
+            return def;
+
+          default:
+            return NULL;
+        }
+      }
+
+      default:
+        return NULL;
+    }
+  }
+
+  return NULL;
+}
+
+// Identity comparison between two distinct concrete entity types can never be
+// true: a value of class C is never the same object as a value of class D, and
+// likewise for any pair drawn from {class, actor, primitive, struct}. Reject
+// such comparisons at compile time.
+//
+// Scope is deliberately narrow:
+//  - Only fires when both operand types reduce to a single TK_NOMINAL bound
+//    to a concrete entity definition.
+//  - Distinct reifications of the same generic class (Array[U64] vs
+//    Array[U32]) share a root entity definition pointer and are NOT flagged.
+//  - Unions, intersections, tuples, type parameters, and trait/interface
+//    operands are skipped to avoid "action at a distance" — a future code
+//    change to a structural type set could legitimately make today's
+//    comparison meaningful.
+static bool verify_not_disjoint_concrete(pass_opt_t* opt, ast_t* ast,
+  ast_t* left, ast_t* right)
+{
+  ast_t* l_type = ast_type(left);
+  ast_t* r_type = ast_type(right);
+
+  pony_assert(l_type != NULL);
+  pony_assert(r_type != NULL);
+
+  // Defensive early-return in release builds where pony_assert elides.
+  if((l_type == NULL) || (r_type == NULL))
+    return true;
+
+  ast_t* l_free = NULL;
+  ast_t* r_free = NULL;
+  ast_t* l_def = concrete_nominal_def(l_type, &l_free);
+  ast_t* r_def = concrete_nominal_def(r_type, &r_free);
+
+  bool ok = true;
+
+  if((l_def != NULL) && (r_def != NULL) && (l_def != r_def))
+  {
+    ast_error(opt->check.errors, ast,
+      "identity comparison between disjoint concrete types %s and %s will"
+      " always be false",
+      ast_print_type(l_type), ast_print_type(r_type));
+    ok = false;
+  }
+
+  if(l_free != NULL)
+    ast_free_unattached(l_free);
+  if(r_free != NULL)
+    ast_free_unattached(r_free);
+
+  return ok;
+}
+
 static bool verify_is(pass_opt_t* opt, ast_t* ast)
 {
   pony_assert((ast_id(ast) == TK_IS) || (ast_id(ast) == TK_ISNT));
   AST_GET_CHILDREN(ast, left, right);
   ast_inheritflags(ast);
-  return verify_is_comparand(opt, right) && verify_is_comparand(opt, left);
+
+  // verify_is_comparand uses && short-circuit: if `right` triggers the
+  // "identity comparison with a new object" error, `left` is not checked,
+  // and verify_not_disjoint_concrete below is not reached. This avoids
+  // double-reporting on a single expression. Do not split or reorder these
+  // calls without preserving that property.
+  if(!(verify_is_comparand(opt, right) && verify_is_comparand(opt, left)))
+    return false;
+
+  return verify_not_disjoint_concrete(opt, ast, left, right);
 }
 
 ast_result_t pass_verify(ast_t** astp, pass_opt_t* options)

test/full-program-tests/identity-numeric-is-numeric/main.pony

@@ -2,8 +2,6 @@ use @pony_exitcode[None](code: I32)
 
 actor Main
   new create(env: Env) =>
-    if (U32(0) is U32(0)) and (U32(0) isnt U32(1)) and
-      (U32(0) isnt U64(0))
-    then
+    if (U32(0) is U32(0)) and (U32(0) isnt U32(1)) then
       @pony_exitcode(1)
     end

test/libponyc/badpony.cc

@@ -1779,6 +1779,199 @@ TEST_F(BadPonyTest, TypeErrorDuringArrayLiteralInference)
   TEST_ERRORS_1(src, "type argument is outside its constraint");
 }
 
+// From issue #1977. Identity comparisons between two distinct concrete
+// entity types can never be true, so reject them at compile time. Scope is
+// deliberately limited to TK_NOMINAL operands bound to concrete entity
+// definitions (class, actor, primitive, struct); unions, tuples, type
+// parameters, and trait/interface operands are intentionally not flagged.
+TEST_F(BadPonyTest, IsBetweenDifferentClasses)
+{
+  const char* src =
+    "class C\n"
+    "class D\n"
+    "actor Main\n"
+    "  new create(env: Env) =>\n"
+    "    let c: C = C\n"
+    "    let d: D = D\n"
+    "    if c is d then None end";
+
+  TEST_ERRORS_1(src,
+    "identity comparison between disjoint concrete types");
+}
+
+TEST_F(BadPonyTest, IsntBetweenDifferentClasses)
+{
+  const char* src =
+    "class C\n"
+    "class D\n"
+    "actor Main\n"
+    "  new create(env: Env) =>\n"
+    "    let c: C = C\n"
+    "    let d: D = D\n"
+    "    if c isnt d then None end";
+
+  TEST_ERRORS_1(src,
+    "identity comparison between disjoint concrete types");
+}
+
+TEST_F(BadPonyTest, IsBetweenClassAndActor)
+{
+  const char* src =
+    "class C\n"
+    "actor A\n"
+    "actor Main\n"
+    "  new create(env: Env) =>\n"
+    "    let c: C = C\n"
+    "    let a: A = A\n"
+    "    if c is a then None end";
+
+  TEST_ERRORS_1(src,
+    "identity comparison between disjoint concrete types");
+}
+
+TEST_F(BadPonyTest, IsBetweenClassAndPrimitive)
+{
+  const char* src =
+    "class C\n"
+    "actor Main\n"
+    "  new create(env: Env) =>\n"
+    "    let c: C = C\n"
+    "    let n: None = None\n"
+    "    if c is n then None end";
+
+  TEST_ERRORS_1(src,
+    "identity comparison between disjoint concrete types");
+}
+
+TEST_F(BadPonyTest, IsBetweenDifferentPrimitives)
+{
+  const char* src =
+    "primitive P\n"
+    "actor Main\n"
+    "  new create(env: Env) =>\n"
+    "    let p: P = P\n"
+    "    let n: None = None\n"
+    "    if p is n then None end";
+
+  TEST_ERRORS_1(src,
+    "identity comparison between disjoint concrete types");
+}
+
+TEST_F(BadPonyTest, IsBetweenClassAndStruct)
+{
+  const char* src =
+    "class C\n"
+    "struct S\n"
+    "actor Main\n"
+    "  new create(env: Env) =>\n"
+    "    let c: C = C\n"
+    "    let s: S = S\n"
+    "    if c is s then None end";
+
+  TEST_ERRORS_1(src,
+    "identity comparison between disjoint concrete types");
+}
+
+TEST_F(BadPonyTest, IsBetweenDifferentNumericPrimitives)
+{
+  // Machine-word numeric types (U8/U16/.../F64) are concrete primitives, so
+  // cross-width identity comparison is statically disjoint and rejected.
+  const char* src =
+    "actor Main\n"
+    "  new create(env: Env) =>\n"
+    "    if U32(0) isnt U64(0) then None end";
+
+  TEST_ERRORS_1(src,
+    "identity comparison between disjoint concrete types");
+}
+
+TEST_F(BadPonyTest, IsBetweenDifferentActors)
+{
+  const char* src =
+    "actor A\n"
+    "actor B\n"
+    "actor Main\n"
+    "  new create(env: Env) =>\n"
+    "    let a: A = A\n"
+    "    let b: B = B\n"
+    "    if a is b then None end";
+
+  TEST_ERRORS_1(src,
+    "identity comparison between disjoint concrete types");
+}
+
+TEST_F(BadPonyTest, IsBetweenDisjointConcreteTypesViaAlias)
+{
+  // Both operands have alias types. The check must unwrap the aliases
+  // before comparing definitions; otherwise the error would not fire.
+  const char* src =
+    "class C\n"
+    "class D\n"
+    "type CA is C\n"
+    "type DA is D\n"
+    "actor Main\n"
+    "  new create(env: Env) =>\n"
+    "    let ca: CA = C\n"
+    "    let da: DA = D\n"
+    "    if ca is da then None end";
+
+  TEST_ERRORS_1(src,
+    "identity comparison between disjoint concrete types");
+}
+
+TEST_F(BadPonyTest, IsBetweenDisjointFieldsThroughThisArrow)
+{
+  // Field reads inside a method body produce viewpoint-adapted types like
+  // `this->C`. The check must unwrap the arrow before classifying the
+  // nominal underneath.
+  const char* src =
+    "class C\n"
+    "class D\n"
+    "class Holder\n"
+    "  let c: C = C\n"
+    "  let d: D = D\n"
+    "  fun ref check(): Bool => c is d\n"
+    "actor Main\n"
+    "  new create(env: Env) => None";
+
+  TEST_ERRORS_1(src,
+    "identity comparison between disjoint concrete types");
+}
+
+TEST_F(BadPonyTest, IsBetweenDisjointClassesViaSugar)
+{
+  // Pony desugars a bare type name in expression position to a default
+  // constructor call (`C` -> `C.create()`). The existing "new object"
+  // diagnostic in verify_is_comparand fires first; the && short-circuit in
+  // verify_is suppresses the new disjoint-concrete check so the user sees
+  // exactly one error per identity comparison, not two.
+  const char* src =
+    "class C\n"
+    "class D\n"
+    "actor Main\n"
+    "  new create(env: Env) =>\n"
+    "    if C is D then None end";
+
+  TEST_ERRORS_1(src,
+    "identity comparison with a new object will always be false");
+}
+
+TEST_F(BadPonyTest, IsBetweenDistinctObjectLiterals)
+{
+  // Per the design decision in #1977: each `object end` literal synthesizes
+  // a distinct anonymous class, so two literals are statically disjoint
+  // concrete types. The comparison can never be true and is rejected.
+  const char* src =
+    "actor Main\n"
+    "  new create(env: Env) =>\n"
+    "    let a = object end\n"
+    "    let b = object end\n"
+    "    if a is b then None end";
+
+  TEST_ERRORS_1(src,
+    "identity comparison between disjoint concrete types");
+}
+
 TEST_F(BadPonyTest, NosupertypeAnnotationProvides)
 {
   const char* src =