From e3f9a33dfdab0a0366f6047f815be9fe261d04d3 Mon Sep 17 00:00:00 2001
From: zerbina <100542850+zerbina@users.noreply.github.com>
Date: Sat, 15 Jun 2024 23:38:49 +0200
Subject: [PATCH] typetraits: improve `genericParams` (#1347)

## Summary

The `genericParams` typetrait now also supports `ref`, `var`, `ptr`,
and `range`. In addition, it's fully based on type information, making
it work regardless of what expression is used as the argument. For
`array` types, the index type is now *always* a `range` type, so this
is a **breaking change**.

## Details

The main goal is removing the mixed analysis from `genericParams`,
where both type and normal AST were inspected. Besides only working for
some type expressions, this also relied on `sem` modifying the input
AST nodes' types in type AST, making the macro susceptible to breaking
when analysis of type AST changes in `sem`.

To make the implementation not rely on `sem` implementation details,
`genericParams` now only uses the macro type API, also removing the
need for an intermediate macro.

The syntax convenience of specifying an array's index type with a
single integer value does not exist at the type level, and a
`StaticParam[x]` is therefore not returned for `array` types anymore.
---
 lib/pure/typetraits.nim                     | 96 ++++++++-------------
 tests/lang_objects/metatype/ttypetraits.nim | 11 ++-
 2 files changed, 46 insertions(+), 61 deletions(-)

diff --git a/lib/pure/typetraits.nim b/lib/pure/typetraits.nim
index 9b3877decfd..d8b93b49d97 100644
--- a/lib/pure/typetraits.nim
+++ b/lib/pure/typetraits.nim
@@ -236,66 +236,12 @@ macro enumLen*(T: typedesc[enum]): int =
   expectKind(enumTy, nnkEnumTy)
   result = newLit(enumTy.len - 1)
 
-macro genericParamsImpl(T: typedesc): untyped =
-  # auxiliary macro needed, can't do it directly in `genericParams`
-  result = newNimNode(nnkTupleConstr)
-  var impl = getTypeImpl(T)
-  expectKind(impl, nnkBracketExpr)
-  impl = impl[1]
-  while true:
-    case impl.kind
-    of nnkSym:
-      impl = impl.getImpl
-    of nnkTypeDef:
-      impl = impl[2]
-    of nnkTypeOfExpr:
-      impl = getTypeInst(impl[0])
-    of nnkBracketExpr:
-      for i in 1..<impl.len:
-        let ai = impl[i]
-        var ret: NimNode = nil
-        case ai.typeKind
-        of ntyTypeDesc:
-          ret = ai
-        of ntyStatic: doAssert false
-        else:
-          # getType from a resolved symbol might return a typedesc symbol.
-          # If so, use it directly instead of wrapping it in StaticParam.
-          if (ai.kind == nnkSym and ai.symKind == nskType) or
-              (ai.kind == nnkBracketExpr and ai[0].kind == nnkSym and
-              ai[0].symKind == nskType) or ai.kind in {nnkRefTy, nnkVarTy, nnkPtrTy, nnkProcTy}:
-            ret = ai
-          elif ai.kind == nnkInfix and ai[0].kind == nnkIdent and
-                ai[0].strVal == "..":
-            # For built-in array types, the "2" is translated to "0..1" then
-            # automagically translated to "range[0..1]". However this is not
-            # reflected in the AST, thus requiring manual transformation here.
-            #
-            # We will also be losing some context here:
-            #   var a: array[10, int]
-            # will be translated to:
-            #   var a: array[0..9, int]
-            # after typecheck. This means that we can't get the exact
-            # definition as typed by the user, which will cause confusion for
-            # users expecting:
-            #   genericParams(typeof(a)) is (StaticParam(10), int)
-            # to be true while in fact the result will be:
-            #   genericParams(typeof(a)) is (range[0..9], int)
-            ret = newTree(nnkBracketExpr, @[bindSym"range", ai])
-          else:
-            since (1, 1):
-              ret = newTree(nnkBracketExpr, @[bindSym"StaticParam", ai])
-        result.add ret
-      break
-    else:
-      error "wrong kind: " & $impl.kind, impl
-
 since (1, 1):
-  template genericParams*(T: typedesc): untyped =
+  macro genericParams*(T: typedesc): untyped =
     ## Returns the tuple of generic parameters for the generic type `T`.
     ##
     ## **Note:** For the builtin array type, the index generic parameter will
-    ## **always** become a range type after it's bound to a variable.
+    ## **always** become a range type.
     runnableExamples:
       type Foo[T1, T2] = object
 
@@ -309,13 +255,43 @@ since (1, 1):
       var s: seq[Bar[3.0, string]]
       doAssert genericParams(typeof(s)) is (Bar[3.0, string],)
 
-      doAssert genericParams(array[10, int]) is (StaticParam[10], int)
+      doAssert genericParams(array[10, int]) is (range[0..9], int)
       var a: array[10, int]
       doAssert genericParams(typeof(a)) is (range[0..9], int)
 
-    type T2 = T
-    genericParamsImpl(T2)
-
+    let desc = getTypeInst(T)
+    expectKind(desc, nnkBracketExpr)
+    let typ = getType(desc[1]) # skip aliases
+
+    result = newNimNode(nnkTupleConstr)
+    case typ.typeKind
+    of ntyGenericInst:
+      # fetch all instnatiation parameters
+      for i in 1..<typ.len:
+        let op = getTypeInst(typ[i])
+        # ``getTypeInst`` loses the staticness, so `typ` has to be queried
+        # instead
+        if typ[i].typeKind == ntyStatic:
+          result.add nnkBracketExpr.newTree(bindSym"StaticParam", op)
+        else:
+          result.add op
+    of ntyPtr, ntyRef, ntyVar, ntySequence, ntyOpenArray, ntyVarargs, ntySet,
+       ntyUncheckedArray:
+      result.add typ[1]
+    of ntyRange:
+      result.add nnkBracketExpr.newTree(bindSym"StaticParam", typ[1])
+      result.add nnkBracketExpr.newTree(bindSym"StaticParam", typ[2])
+    of ntyArray:
+      var len = getTypeInst(typ[1])
+      if len.kind == nnkInfix:
+        # create a proper range type constructor
+        len = nnkBracketExpr.newTree(bindSym"range", len)
+
+      result = nnkTupleConstr.newTree(
+        len,
+        typ[2])
+    else:
+      error("not an instantiated generic type", T)
 
 proc hasClosureImpl(n: NimNode): bool = discard "see compiler/vmops.nim"
 
diff --git a/tests/lang_objects/metatype/ttypetraits.nim b/tests/lang_objects/metatype/ttypetraits.nim
index 631615dd5a9..c4b24957b1a 100644
--- a/tests/lang_objects/metatype/ttypetraits.nim
+++ b/tests/lang_objects/metatype/ttypetraits.nim
@@ -245,9 +245,18 @@ block genericParams:
 
   block nestedContainers:
     doAssert genericParams(seq[Foo[string, float]]).get(0) is Foo[string, float]
-    doAssert genericParams(array[10, Foo[Bar[1, int], Bar[2, float]]]) is (StaticParam[10], Foo[Bar[1, int], Bar[2, float]])
+    doAssert genericParams(array[10, Foo[Bar[1, int], Bar[2, float]]]) is (range[0..9], Foo[Bar[1, int], Bar[2, float]])
     doAssert genericParams(array[1..9, int]) is (range[1..9], int)
 
+  doAssert genericParams(var int) is (int,)
+  doAssert genericParams(ptr int) is (int,)
+  doAssert genericParams(ref int) is (int,)
+  doAssert genericParams(set[int16]) is (int16,)
+  doAssert genericParams(openArray[int]) is (int,)
+  doAssert genericParams(varargs[int]) is (int,)
+  doAssert genericParams(UncheckedArray[int]) is (int,)
+  doAssert genericParams(range[1 .. 2]) is (StaticParam[1], StaticParam[2])
+
 ##############################################
 # bug 13095