add isNamedTuple; make $(1, 2) be (1, 2) instead of (Field0: 1, Field1: 2) which leaked implementation detail (#10070)

* add `isNamedTuple`; make $(1, 2) be (1, 2) instead of leaking implementation detail (Field0: 1, Field1: 2)
  fixes this: #8670 (comment) /cc @alehander42 @Vindaar @mratsim

* Note: isNamedTuple is useful in other places, eg #10010 (comment)

* move isNamedTuple to helpers.nim to avoid exposing new symbol to system.nim

* remove workaround in tests/vm/tissues.nim failing test now that #10218 was makes it work

lib/pure/typetraits.nim

@@ -10,7 +10,10 @@
 ## This module defines compile-time reflection procs for
 ## working with types
 
+include "system/helpers" # for `isNamedTuple`
+
 export system.`$`
+export isNamedTuple
 
 proc name*(t: typedesc): string {.magic: "TypeTrait".}
   ## Alias for system.`$`(t) since Nim v0.20.0.

lib/system.nim

@@ -2669,19 +2669,28 @@ proc compiles*(x: untyped): bool {.magic: "Compiles", noSideEffect, compileTime.
   ##     echo "'+' for integers is available"
   discard
 
+include "system/helpers" # for `lineInfoToString`, `isNamedTuple`
+
 proc `$`*[T: tuple|object](x: T): string =
   ## generic ``$`` operator for tuples that is lifted from the components
   ## of `x`. Example:
   ##
   ## .. code-block:: nim
-  ##   $(23, 45) == "(Field0: 23, Field1: 45)"
+  ##   $(23, 45) == "(23, 45)"
+  ##   $(a: 23, b: 45) == "(a: 23, b: 45)"
   ##   $() == "()"
   result = "("
   var firstElement = true
+  const isNamed = T is object or isNamedTuple(T)
+  when not isNamed:
+    var count = 0
   for name, value in fieldPairs(x):
     if not firstElement: result.add(", ")
-    result.add(name)
-    result.add(": ")
+    when isNamed:
+      result.add(name)
+      result.add(": ")
+    else:
+      count.inc
     when compiles($value):
       when value isnot string and value isnot seq and compiles(value.isNil):
         if value.isNil: result.add "nil"
@@ -2691,6 +2700,10 @@ proc `$`*[T: tuple|object](x: T): string =
       firstElement = false
     else:
       result.add("...")
+  when not isNamed:
+    if count == 1:
+      result.add(",") # $(1,) should print as the semantically legal (1,)
+
   result.add(")")
 
 proc collectionToString[T](x: T, prefix, separator, suffix: string): string =
@@ -3958,8 +3971,6 @@ proc failedAssertImpl*(msg: string) {.raises: [], tags: [].} =
                                     tags: [].}
   Hide(raiseAssert)(msg)
 
-include "system/helpers" # for `lineInfoToString`
-
 template assertImpl(cond: bool, msg: string, expr: string, enabled: static[bool]) =
   const loc = $instantiationInfo(-1, true)
   bind instantiationInfo

lib/system/helpers.nim

@@ -6,6 +6,23 @@
 proc lineInfoToString(file: string, line, column: int): string =
   file & "(" & $line & ", " & $column & ")"
 
-proc `$`(info: type(instantiationInfo(0))): string =
+type InstantiationInfo = tuple[filename: string, line: int, column: int]
+
+proc `$`(info: InstantiationInfo): string =
   # The +1 is needed here
+  # instead of overriding `$` (and changing its meaning), consider explicit name.
   lineInfoToString(info.fileName, info.line, info.column+1)
+
+proc isNamedTuple(T: type): bool =
+  ## return true for named tuples, false for any other type.
+  when T isnot tuple: result = false
+  else:
+    var t: T
+    for name, _ in t.fieldPairs:
+      when name == "Field0":
+        return compiles(t.Field0)
+      else:
+        return true
+    # empty tuple should be un-named,
+    # see https://github.com/nim-lang/Nim/issues/8861#issue-356631191
+    return false

lib/system/helpers2.nim

@@ -1,3 +1,5 @@
+# imported by other modules, unlike helpers.nim which is included
+
 template formatErrorIndexBound*[T](i, a, b: T): string =
   "index out of bounds: (a:" & $a & ") <= (i:" & $i & ") <= (b:" & $b & ") "
 

tests/array/tarray.nim

@@ -18,7 +18,7 @@ paper
 @[2, 3, 4]321
 9.0 4.0
 3
-@[(Field0: 1, Field1: 2), (Field0: 3, Field1: 5)]
+@[(1, 2), (3, 5)]
 2
 @["a", "new one", "c"]
 @[1, 2, 3]

tests/ccgbugs/t5701.nim

@@ -1,7 +1,7 @@
 discard """
-  output: '''(Field0: 1, Field1: 1)
-(Field0: 2, Field1: 2)
-(Field0: 3, Field1: 3)
+  output: '''(1, 1)
+(2, 2)
+(3, 3)
 '''
 """
 

tests/collections/tcollections_to_string.nim

@@ -9,9 +9,9 @@ import lists
 import critbits
 
 # Tests for tuples
-doAssert $(1, 2, 3) == "(Field0: 1, Field1: 2, Field2: 3)"
-doAssert $("1", "2", "3") == """(Field0: "1", Field1: "2", Field2: "3")"""
-doAssert $('1', '2', '3') == """(Field0: '1', Field1: '2', Field2: '3')"""
+doAssert $(1, 2, 3) == "(1, 2, 3)"
+doAssert $("1", "2", "3") == """("1", "2", "3")"""
+doAssert $('1', '2', '3') == """('1', '2', '3')"""
 
 # Tests for seqs
 doAssert $(@[1, 2, 3]) == "@[1, 2, 3]"

tests/errmsgs/tnested_generic_instantiation.nim

@@ -17,3 +17,9 @@ converter toWrapped[T](value: T): Wrapped[T] =
 
 let result = Plain()
 discard $result
+
+proc foo[T2](a: Wrapped[T2]) =
+  # Error: generic instantiation too nested
+  discard $a
+
+foo(result)

tests/errmsgs/tnested_generic_instantiation2.nim

@@ -0,0 +1,27 @@
+discard """
+errormsg: "generic instantiation too nested"
+"""
+
+#[
+bug #4766
+see also: tnested_generic_instantiation.nim
+]#
+
+proc toString*[T](x: T) =
+  for name, value in fieldPairs(x):
+    when compiles(toString(value)):
+      discard
+    toString(value)
+
+type
+  Plain = ref object
+    discard
+
+  Wrapped[T] = object
+    value: T
+
+converter toWrapped[T](value: T): Wrapped[T] =
+  Wrapped[T](value: value)
+
+let result = Plain()
+toString(result)

tests/generics/treentranttypes.nim

@@ -1,6 +1,6 @@
 discard """
 output: '''
-(Field0: 10, Field1: (Field0: "test", Field1: 1.2))
+(10, ("test", 1.2))
 3x3 Matrix [[0.0, 2.0, 3.0], [2.0, 0.0, 5.0], [2.0, 0.0, 5.0]]
 
 2x3 Matrix [[0.0, 2.0, 3.0], [2.0, 0.0, 5.0]]

tests/metatype/tmetatype_issues.nim

@@ -1,7 +1,7 @@
 discard """
 output:'''
 void
-(Field0: "string", Field1: "string")
+("string", "string")
 1 mod 7
 @[2, 2, 2, 2, 2]
 impl 2 called

tests/metatype/ttypetraits2.nim

@@ -1,3 +1,18 @@
 # todo: merge with $nimc_D/tests/metatype/ttypetraits.nim (currently disabled)
 
 from typetraits import `$` # checks fix for https://github.com/c-blake/cligen/issues/84
+
+import typetraits
+
+block: # isNamedTuple
+  type Foo1 = (a:1,).type
+  type Foo2 = (Field0:1,).type
+  type Foo3 = ().type
+  type Foo4 = object
+
+  doAssert (a:1,).type.isNamedTuple
+  doAssert Foo1.isNamedTuple
+  doAssert Foo2.isNamedTuple
+  doAssert not Foo3.isNamedTuple
+  doAssert not Foo4.isNamedTuple
+  doAssert not (1,).type.isNamedTuple

tests/system/tsystem_misc.nim

@@ -148,3 +148,14 @@ let a = @[1, 2, 3]
 
 # a.boundedOpenArray(1, 2).foo()  # Works
 echo a.boundedOpenArray(1, 2).len # Internal compiler error
+
+block: # `$`*[T: tuple|object](x: T)
+  doAssert $(foo1:0, bar1:"a") == """(foo1: 0, bar1: "a")"""
+  doAssert $(foo1:0, ) == """(foo1: 0)"""
+  doAssert $(0, "a") == """(0, "a")"""
+  doAssert $(0, ) == "(0,)"
+  type Foo = object
+    x:int
+    x2:float
+  doAssert $Foo(x:2) == "(x: 2, x2: 0.0)"
+  doAssert $() == "()"

tests/vm/tissues.nim

@@ -1,16 +1,14 @@
-discard """
-  nimout: "(Field0: 2, Field1: 2, Field2: 2, Field3: 2)"
-"""
-
 import macros
 
-block t9043:
-  proc foo[N: static[int]](dims: array[N, int])=
+block t9043: # issue #9043
+  proc foo[N: static[int]](dims: array[N, int]): string =
     const N1 = N
     const N2 = dims.len
-    static: echo (N, dims.len, N1, N2)
+    const ret = $(N, dims.len, N1, N2)
+    static: doAssert ret == $(N, dims.len, N1, N2)
+    ret
 
-  foo([1, 2])
+  doAssert foo([1, 2]) == "(2, 2, 2, 2)"
 
 block t4952:
   proc doCheck(tree: NimNode) =