Converting derived type to distinct base type crashes compiler

[bptato]

When converting a derived ref object to a distinct type of its base type, the compiler asserts.

Example

type
  A = ref object of RootObj
  B = ref object of A
  C = distinct A

discard C(B())

Actual Output

/src/nimskull/compiler/nim.nim(156) nim
/src/nimskull/compiler/nim.nim(91) handleCmdLine
/src/nimskull/compiler/front/main.nim(541) mainCommand
/src/nimskull/compiler/front/main.nim(493) compileToBackend
/src/nimskull/compiler/front/main.nim(219) commandCompileToC
/src/nimskull/compiler/backend/cbackend.nim(409) generateCode
/src/nimskull/compiler/backend/backends.nim(815) generateCode
/src/nimskull/compiler/backend/backends.nim(397) translate
/src/nimskull/compiler/mir/mirgen.nim(2552) generateCode
/src/nimskull/compiler/mir/mirgen.nim(2302) gen
/src/nimskull/compiler/mir/mirgen.nim(2365) gen
/src/nimskull/compiler/mir/mirgen.nim(411) exprToPmir
/src/nimskull/compiler/mir/proto_mir.nim(819) exprToPmir
/src/nimskull/compiler/mir/proto_mir.nim(672) exprToPmir
/src/nimskull/compiler/mir/proto_mir.nim(496) analyseObjConv
/src/nimskull/compiler/ast/types.nim(922) inheritanceDiff
/src/nimskull/lib/system/assertions.nim(35) failedAssertImpl
/src/nimskull/lib/system/assertions.nim(25) raiseAssert
/src/nimskull/lib/system/fatal.nim(50) sysFatal
Error: unhandled exception: /src/nimskull/compiler/ast/types.nim(922, 10) `a.kind in {tyObject} + skipPtrs`  [AssertionDefect]

Expected Output

Compiles.

Possible Solution

I don't know if it's a good solution, but skipping tyDistinct seems to work:

diff --git a/compiler/ast/types.nim b/compiler/ast/types.nim
index 0db66576e..96d685a7c 100644
--- a/compiler/ast/types.nim
+++ b/compiler/ast/types.nim
@@ -919,19 +919,19 @@ proc inheritanceDiff*(a, b: PType): int =
   # | returns: +x iff `a` is the x'th direct subclass of `b`
   # | returns: `maxint` iff `a` and `b` are not compatible at all
   if a == b or a.kind == tyError or b.kind == tyError: return 0
-  assert a.kind in {tyObject} + skipPtrs
-  assert b.kind in {tyObject} + skipPtrs
+  assert a.kind in {tyObject, tyDistinct} + skipPtrs
+  assert b.kind in {tyObject, tyDistinct} + skipPtrs
   var x = a
   result = 0
   while x != nil:
-    x = skipTypes(x, skipPtrs)
+    x = skipTypes(x, skipPtrs + {tyDistinct})
     if sameObjectTypes(x, b): return
     x = x[0]
     inc(result)
   var y = b
   result = 0
   while y != nil:
-    y = skipTypes(y, skipPtrs)
+    y = skipTypes(y, skipPtrs + {tyDistinct})
     if sameObjectTypes(y, a): return
     y = y[0]
     dec(result)

Additional Information

References

[zerbina]

Thank you for reporting the issue.

inheritanceDiff shouldn't skip tyDistinct, as the procedure is responsible for reporting the relation between two object (or ref/ptr to object) types, and A and distinct A don't have a inheritance-related relationship.

The fundamental problem is that C(B()) actually performs two different types of conversion at once: a ref object up-conversion and a to-distinct conversion (in that order). However, transf, which transforms the nkConv into an nkObjUpConv node, assumes that all explicit conversions only ever represent one type of conversion at the same time.

Either two-step conversions should be disallowed at the language level¹, or semantic analysis should split them into two.

For splitting two-step conversions into two, semexprs.semConv would simply insert a hidden conversion (nkHiddenSubConv, in this case) around the conversion source expression, meaning that the typed AST would look like this: (Conv (...) (HiddenSubConv (...) ...)). This way, nkConv always represents a single conversion.

Footnotes

1. In my opinion, at least explicit conversion where reaching the target type requires a to-distinct-base + other conversion (in that order) should be disallowed, as it effectively breaks the distinct abstraction. ↩

[bptato]

Good point. In my case, the distinct type is actually a hack for representing a type defined in a parent module, so the conversion seemed natural to me... but on a type system level, it does seem wrong, so I agree that erroring out in the example is cleaner.

But then, I think there's another bug here:

type
  A = ref object of RootObj
  B = ref object of A
  C = distinct A

# asserts, despite doing conversions separately:
discard C(A(B()))

# curiously, it does work if I add a temporary:
let x = A(B())
discard C(x)

[zerbina]

But then, I think there's another bug here:

Good catch, that's indeed a separate bug.

What happens is that transformation of conversions in transf.transformConv ignores/skips distinct types. When the conversion's source and target types are the same object type (module distinct), the conversion is "optimized" away, with the inner expression's type being changed to the conversion's target type, meaning that the AST reaching proto-MIR translation (where the problematic inheritanceDiff is called from) is effectively the same as if you'd have used C(B()).

This is also why C(x) doesn't cause the assertion to trigger: the expression is turned into just x (which is quite problematic).

I'll make a fix.

[zerbina]

@bptato: Your issue with C(A(B())) also crashing the compiler should be gone when using the most recent compiler version (refer to #1606).

[bptato]

Indeed it is, thank you.