enable vtable implementation for C++ and make it an experimental feature (#23004)

follow up https://github.com/nim-lang/Nim/pull/22991

- [x] turning it into an experimental feature

---------

Co-authored-by: Andreas Rumpf <rumpf_a@web.de>

changelog.md

@@ -5,7 +5,7 @@
 
 - `-d:nimStrictDelete` becomes the default. An index error is produced when the index passed to `system.delete` was out of bounds. Use `-d:nimAuditDelete` to mimic the old behavior for backwards compatibility.
 - The default user-agent in `std/httpclient` has been changed to `Nim-httpclient/<version>` instead of `Nim httpclient/<version>` which was incorrect according to the HTTP spec.
-- Methods now support implementations based on vtable by using `-d:nimPreviewVtables`. methods are confined in the same module where the type has been defined.
+- Methods now support implementations based on a VTable by using `--experimental:vtables`. Methods are then confined to be in the same module where their type has been defined.
 - With `-d:nimPreviewNonVarDestructor`, non-var destructors become the default.
 
 ## Standard library additions and changes

compiler/ccgtypes.nim

@@ -1678,6 +1678,13 @@ proc genDisplay(m: BModule; t: PType, depth: int): Rope =
   result.add seqs[0]
   result.add "}"
 
+proc genVTable(seqs: seq[PSym]): string =
+  result = "{"
+  for i in 0..<seqs.len:
+    if i > 0: result.add ", "
+    result.add "(void *) " & seqs[i].loc.r
+  result.add "}"
+
 proc genTypeInfoV2OldImpl(m: BModule; t, origType: PType, name: Rope; info: TLineInfo) =
   cgsym(m, "TNimTypeV2")
   m.s[cfsStrData].addf("N_LIB_PRIVATE TNimTypeV2 $1;$n", [name])
@@ -1714,18 +1721,19 @@ proc genTypeInfoV2OldImpl(m: BModule; t, origType: PType, name: Rope; info: TLin
     m.s[cfsVars].addf("static $1 $2[$3] = $4;$n", [getTypeDesc(m, getSysType(m.g.graph, unknownLineInfo, tyUInt32), dkVar), objDisplayStore, rope(objDepth+1), objDisplay])
     addf(typeEntry, "$1.display = $2;$n", [name, rope(objDisplayStore)])
 
+  let dispatchMethods = toSeq(getMethodsPerType(m.g.graph, t))
+  if dispatchMethods.len > 0:
+    let vTablePointerName = getTempName(m)
+    m.s[cfsVars].addf("static void* $1[$2] = $3;$n", [vTablePointerName, rope(dispatchMethods.len), genVTable(dispatchMethods)])
+    for i in dispatchMethods:
+      genProcPrototype(m, i)
+    addf(typeEntry, "$1.vTable = $2;$n", [name, vTablePointerName])
+
   m.s[cfsTypeInit3].add typeEntry
 
   if t.kind == tyObject and t.len > 0 and t[0] != nil and optEnableDeepCopy in m.config.globalOptions:
     discard genTypeInfoV1(m, t, info)
 
-proc genVTable(seqs: seq[PSym]): string =
-  result = "{"
-  for i in 0..<seqs.len:
-    if i > 0: result.add ", "
-    result.add "(void *) " & seqs[i].loc.r
-  result.add "}"
-
 proc genTypeInfoV2Impl(m: BModule; t, origType: PType, name: Rope; info: TLineInfo) =
   cgsym(m, "TNimTypeV2")
   m.s[cfsStrData].addf("N_LIB_PRIVATE TNimTypeV2 $1;$n", [name])

compiler/cgen.nim

@@ -2234,8 +2234,7 @@ proc finalCodegenActions*(graph: ModuleGraph; m: BModule; n: PNode) =
         incl m.flags, objHasKidsValid
       if optMultiMethods in m.g.config.globalOptions or
           m.g.config.selectedGC notin {gcArc, gcOrc, gcAtomicArc} or
-          not m.g.config.isDefined("nimPreviewVtables") or
-          m.g.config.backend == backendCpp or sfCompileToCpp in m.module.flags:
+          vtables notin m.g.config.features:
         generateIfMethodDispatchers(graph, m.idgen)
 
 

compiler/cgmeth.nim

@@ -157,7 +157,7 @@ proc fixupDispatcher(meth, disp: PSym; conf: ConfigRef) =
 
 proc methodDef*(g: ModuleGraph; idgen: IdGenerator; s: PSym) =
   var witness: PSym = nil
-  if s.typ[1].owner.getModule != s.getModule and g.config.isDefined("nimPreviewVtables"):
+  if s.typ[1].owner.getModule != s.getModule and vtables in g.config.features and not g.config.isDefined("nimInternalNonVtablesTesting"):
     localError(g.config, s.info, errGenerated, "method `" & s.name.s &
           "` can be defined only in the same module with its type (" & s.typ[1].typeToString() & ")")
   for i in 0..<g.methods.len:

compiler/condsyms.nim

@@ -163,3 +163,5 @@ proc initDefines*(symbols: StringTableRef) =
 
   defineSymbol("nimHasCastExtendedVm")
   defineSymbol("nimHasWarnStdPrefix")
+
+  defineSymbol("nimHasVtables")

compiler/nim.cfg

@@ -9,7 +9,6 @@ define:nimPreviewSlimSystem
 define:nimPreviewCstringConversion
 define:nimPreviewProcConversion
 define:nimPreviewRangeDefault
-define:nimPreviewVtables
 define:nimPreviewNonVarDestructor
 threads:off
 
@@ -58,3 +57,7 @@ define:useStdoutAsStdmsg
   warning[StdPrefix]:on
   warningAsError[StdPrefix]:on
 @end
+
+@if nimHasVtables:
+  experimental:vtables
+@end

compiler/options.nim

@@ -226,7 +226,8 @@ type
     flexibleOptionalParams,
     strictDefs,
     strictCaseObjects,
-    inferGenericTypes
+    inferGenericTypes,
+    vtables
 
   LegacyFeature* = enum
     allowSemcheckedAstModification,

compiler/sem.nim

@@ -861,9 +861,7 @@ proc semStmtAndGenerateGenerics(c: PContext, n: PNode): PNode =
   trackStmt(c, c.module, result, isTopLevel = true)
   if optMultiMethods notin c.config.globalOptions and
       c.config.selectedGC in {gcArc, gcOrc, gcAtomicArc} and
-      c.config.isDefined("nimPreviewVtables") and 
-      c.config.backend != backendCpp and
-      sfCompileToCpp notin c.module.flags:
+      Feature.vtables in c.config.features:
     sortVTableDispatchers(c.graph)
 
     if sfMainModule in c.module.flags:

lib/system.nim

@@ -1606,8 +1606,11 @@ when not defined(js) and defined(nimV2):
       traceImpl: pointer
       typeInfoV1: pointer # for backwards compat, usually nil
       flags: int
-      when defined(nimPreviewVtables) and not defined(cpp):
-        vTable: UncheckedArray[pointer] # vtable for types
+      when defined(gcDestructors):
+        when defined(cpp):
+          vTable: ptr UncheckedArray[pointer] # vtable for types
+        else:
+          vTable: UncheckedArray[pointer] # vtable for types
     PNimTypeV2 = ptr TNimTypeV2
 
 proc supportsCopyMem(t: typedesc): bool {.magic: "TypeTrait".}

lib/system/arc.nim

@@ -244,7 +244,7 @@ template tearDownForeignThreadGc* =
 proc isObjDisplayCheck(source: PNimTypeV2, targetDepth: int16, token: uint32): bool {.compilerRtl, inl.} =
   result = targetDepth <= source.depth and source.display[targetDepth] == token
 
-when defined(nimPreviewVtables) and not defined(cpp):
+when defined(gcDestructors):
   proc nimGetVTable(p: pointer, index: int): pointer
         {.compilerRtl, inline, raises: [].} =
     result = cast[ptr PNimTypeV2](p).vTable[index]

tests/config.nims

@@ -44,5 +44,4 @@ switch("define", "nimPreviewNonVarDestructor")
 
 switch("warningAserror", "UnnamedBreak")
 switch("legacy", "verboseTypeMismatch")
-switch("define", "nimPreviewVtables")
-
+switch("experimental", "vtables")

tests/generics/tobjecttyperel.nim

@@ -1,5 +1,5 @@
 discard """
-  matrix: "-u:nimPreviewVtables"
+  matrix: "-d:nimInternalNonVtablesTesting"
   output: '''(peel: 0, color: 15)
 (color: 15)
 17

tests/method/tgeneric_methods.nim

@@ -1,5 +1,5 @@
 discard """
-  matrix: "--mm:arc --multimethods:on -u:nimPreviewVtables; --mm:refc --multimethods:on -u:nimPreviewVtables"
+  matrix: "--mm:arc --multimethods:on -d:nimInternalNonVtablesTesting; --mm:refc --multimethods:on -d:nimInternalNonVtablesTesting"
   output: '''wow2
 X 1
 X 3'''

tests/method/tmethods_old.nim

@@ -1,5 +1,5 @@
 discard """
-  matrix: "--mm:arc -u:nimPreviewVtables"
+  matrix: "--mm:arc -d:nimInternalNonVtablesTesting"
   output: '''
 do nothing
 '''

tests/method/tvtable.nim

@@ -1,3 +1,7 @@
+discard """
+  targets: "c cpp"
+"""
+
 type FooBase = ref object of RootObj
   dummy: int
 type Foo = ref object of FooBase
@@ -15,5 +19,6 @@ method bar(x: Foo2, a: float32)  =
 proc test() =
   var x = new Foo2
   x.bar(2.3)
+  doAssert x.value <= 2.3
 
 test()