From ab189620857a9fe36480044732c407dfcfdec73a Mon Sep 17 00:00:00 2001 From: metagn Date: Tue, 20 Aug 2024 22:31:19 +0300 Subject: [PATCH] sem all call nodes in generic type bodies + many required fixes (#23983) fixes #23406, closes #23854, closes #23855 (test code of both compiles but separate issue exists), refs #23432, follows #23411 In generic bodies, previously all regular `nkCall` nodes like `foo(a, b)` were directly treated as generic statements and delayed immediately, but other call kinds like `a.foo(b)`, `foo a, b` etc underwent typechecking before making sure they have to be delayed, as implemented in #22029. Since the behavior for `nkCall` was slightly buggy (as in #23406), the behavior for all call kinds is now to call `semTypeExpr`. However the vast majority of calls in generic bodies out there are `nkCall`, and while there isn't a difference in the expected behavior, this exposes many issues with the implementation started in #22029 given how much more code uses it now. The portion of these issues that CI has caught are fixed in this PR but it's possible there are more. 1. Deref expressions, dot expressions and calls to dot expressions now handle and propagate `tyFromExpr`. This is most of the changes in `semexprs`. 2. For deref expressions to work in `typeof`, a new type flag `tfNonConstExpr` is added for `tyFromExpr` that calls `semExprWithType` with `efInTypeof` on the expression instead of `semConstExpr`. This type flag is set for every `tyFromExpr` type of a node that `prepareNode` encounters, so that the node itself isn't evaluated at compile time when just trying to get the type of the node. 3. Unresolved `static` types matching `static` parameters is now treated the same as unresolved generic types matching `typedesc` parameters in generic type bodies, it causes a failed match which delays the call instantiation. 4. `typedesc` parameters now reject all types containing unresolved generic types like `seq[T]`, not just generic param types by themselves. (using `containsGenericType`) 5. `semgnrc` now doesn't leave generic param symbols it encounters in generic type contexts as just identifiers, and instead turns them into symbol nodes. Normally in generic procs, this isn't a problem since the generic param symbols will be provided again at instantiation time (and in fact creating symbol nodes causes issues since `seminst` doesn't actually instantiate proc body node types). But generic types can try to be instantiated early in `sigmatch` which will give an undeclared identifier error when the param is not provided. Nodes in generic types (specifically in `tyFromExpr` which should be the only use for `semGenericStmt`) undergo full generic type instantiation with `prepareNode`, so there is no issue of these symbols remaining as uninstantiated generic types. 6. `prepareNode` now has more logic for which nodes to avoid instantiating. Subscripts and subscripts turned into calls to `[]` by `semgnrc` need to avoid instantiating the first operand, since it may be a generic body type like `Generic` in an expression like `Generic[int]`. Dot expressions cannot instantiate their RHS as it may be a generic proc symbol or even an undeclared identifier for generic param fields, but have to instantiate their LHS, so calls and subscripts need to still instantiate their first node if it's a dot expression. This logic still isn't perfect and needs the same level of detail as in `semexprs` for which nodes can be left as "untyped" for overloading/dot exprs/subscripts to handle, but should handle the majority of cases. Also the `efDetermineType` requirement for which calls become `tyFromExpr` is removed and as a result `efDetermineType` is entirely unused again. --- compiler/ast.nim | 2 + compiler/semcall.nim | 2 +- compiler/semexprs.nim | 34 ++++++--- compiler/semgnrc.nim | 24 ++++++ compiler/semtypes.nim | 10 +-- compiler/semtypinst.nim | 74 +++++++++++++++--- compiler/sigmatch.nim | 10 ++- tests/generics/t23854.nim | 70 +++++++++++++++++ tests/generics/t23855.nim | 61 +++++++++++++++ tests/generics/tcalltype.nim | 26 +++++++ .../generics/tuninstantiatedgenericcalls.nim | 76 +++++++++++++++++++ tests/metatype/ttypedescnotnimnode.nim | 21 +++++ tests/metatype/ttypeselectors.nim | 13 ---- 13 files changed, 379 insertions(+), 44 deletions(-) create mode 100644 tests/generics/t23854.nim create mode 100644 tests/generics/t23855.nim create mode 100644 tests/generics/tcalltype.nim create mode 100644 tests/metatype/ttypedescnotnimnode.nim diff --git a/compiler/ast.nim b/compiler/ast.nim index 648bc4392857a..d0d4f8db367f5 100644 --- a/compiler/ast.nim +++ b/compiler/ast.nim @@ -448,6 +448,8 @@ const tfGcSafe* = tfThread tfObjHasKids* = tfEnumHasHoles tfReturnsNew* = tfInheritable + tfNonConstExpr* = tfExplicitCallConv + ## tyFromExpr where the expression shouldn't be evaluated as a static value skError* = skUnknown var diff --git a/compiler/semcall.nim b/compiler/semcall.nim index a136cf4fe9d42..3981d256fd710 100644 --- a/compiler/semcall.nim +++ b/compiler/semcall.nim @@ -751,7 +751,7 @@ proc semOverloadedCall(c: PContext, n, nOrig: PNode, candidates) result = semResolvedCall(c, r, n, flags, expectedType) else: - if efDetermineType in flags and c.inGenericContext > 0 and c.matchedConcept == nil: + if c.inGenericContext > 0 and c.matchedConcept == nil: result = semGenericStmt(c, n) result.typ = makeTypeFromExpr(c, result.copyTree) elif efExplain notin flags: diff --git a/compiler/semexprs.nim b/compiler/semexprs.nim index e812aa1f6814b..bc66ba3d50706 100644 --- a/compiler/semexprs.nim +++ b/compiler/semexprs.nim @@ -1028,7 +1028,7 @@ proc semOverloadedCallAnalyseEffects(c: PContext, n: PNode, nOrig: PNode, if result != nil: if result[0].kind != nkSym: - if not (efDetermineType in flags and c.inGenericContext > 0): + if not (c.inGenericContext > 0): # see generic context check in semOverloadedCall internalError(c.config, "semOverloadedCallAnalyseEffects") return let callee = result[0].sym @@ -1137,6 +1137,11 @@ proc semIndirectOp(c: PContext, n: PNode, flags: TExprFlags; expectedType: PType result.flags.incl nfExplicitCall for i in 1.. 0: + # don't make assumptions, entire expression needs to be tyFromExpr + result = semGenericStmt(c, n) + result.typ = makeTypeFromExpr(c, result.copyTree) + return else: n[0] = n0 else: @@ -1480,17 +1485,17 @@ proc tryReadingGenericParam(c: PContext, n: PNode, i: PIdent, t: PType): PNode = of tyTypeParamsHolders: result = readTypeParameter(c, t, i, n.info) if result == c.graph.emptyNode: - result = n - n.typ = makeTypeFromExpr(c, n.copyTree) + result = semGenericStmt(c, n) + result.typ = makeTypeFromExpr(c, result.copyTree) of tyUserTypeClasses: if t.isResolvedUserTypeClass: result = readTypeParameter(c, t, i, n.info) else: - n.typ = makeTypeFromExpr(c, copyTree(n)) - result = n - of tyGenericParam, tyAnything: - n.typ = makeTypeFromExpr(c, copyTree(n)) - result = n + result = semGenericStmt(c, n) + result.typ = makeTypeFromExpr(c, copyTree(result)) + of tyFromExpr, tyGenericParam, tyAnything: + result = semGenericStmt(c, n) + result.typ = makeTypeFromExpr(c, copyTree(result)) else: result = nil @@ -1654,18 +1659,20 @@ proc buildOverloadedSubscripts(n: PNode, ident: PIdent): PNode = result.add(newIdentNode(ident, n.info)) for s in n: result.add s -proc semDeref(c: PContext, n: PNode): PNode = +proc semDeref(c: PContext, n: PNode, flags: TExprFlags): PNode = checkSonsLen(n, 1, c.config) n[0] = semExprWithType(c, n[0]) let a = getConstExpr(c.module, n[0], c.idgen, c.graph) if a != nil: - if a.kind == nkNilLit: + if a.kind == nkNilLit and efInTypeof notin flags: localError(c.config, n.info, "nil dereference is not allowed") n[0] = a result = n var t = skipTypes(n[0].typ, {tyGenericInst, tyVar, tyLent, tyAlias, tySink, tyOwned}) case t.kind of tyRef, tyPtr: n.typ = t.elementType + of tyMetaTypes, tyFromExpr: + n.typ = makeTypeFromExpr(c, n.copyTree) else: result = nil #GlobalError(n[0].info, errCircumNeedsPointer) @@ -1696,8 +1703,11 @@ proc semSubscript(c: PContext, n: PNode, flags: TExprFlags): PNode = ## checking of assignments result = nil if n.len == 1: - let x = semDeref(c, n) + let x = semDeref(c, n, flags) if x == nil: return nil + if x.typ.kind == tyFromExpr: + # depends on generic type + return x result = newNodeIT(nkDerefExpr, x.info, x.typ) result.add(x[0]) return @@ -3394,7 +3404,7 @@ proc semExpr(c: PContext, n: PNode, flags: TExprFlags = {}, expectedType: PType result = semArrayConstr(c, n, flags, expectedType) of nkObjConstr: result = semObjConstr(c, n, flags, expectedType) of nkLambdaKinds: result = semProcAux(c, n, skProc, lambdaPragmas, flags) - of nkDerefExpr: result = semDeref(c, n) + of nkDerefExpr: result = semDeref(c, n, flags) of nkAddr: result = n checkSonsLen(n, 1, c.config) diff --git a/compiler/semgnrc.nim b/compiler/semgnrc.nim index e2da56c5d7abd..e7f36879f0f61 100644 --- a/compiler/semgnrc.nim +++ b/compiler/semgnrc.nim @@ -104,6 +104,18 @@ proc semGenericStmtSymbol(c: PContext, n: PNode, s: PSym, if s.typ != nil and s.typ.kind == tyStatic: if s.typ.n != nil: result = s.typ.n + elif c.inGenericContext > 0 and withinConcept notin flags: + # don't leave generic param as identifier node in generic type, + # sigmatch will try to instantiate generic type AST without all params + # fine to give a symbol node a generic type here since + # we are in a generic context and `prepareNode` will be called + result = newSymNodeTypeDesc(s, c.idgen, n.info) + if canOpenSym(result.sym): + if genericsOpenSym in c.features: + result = newOpenSym(result) + else: + result.flags.incl nfDisabledOpenSym + result.typ = nil else: result = n else: @@ -128,6 +140,18 @@ proc semGenericStmtSymbol(c: PContext, n: PNode, s: PSym, else: result.flags.incl nfDisabledOpenSym result.typ = nil + elif c.inGenericContext > 0 and withinConcept notin flags: + # don't leave generic param as identifier node in generic type, + # sigmatch will try to instantiate generic type AST without all params + # fine to give a symbol node a generic type here since + # we are in a generic context and `prepareNode` will be called + result = newSymNodeTypeDesc(s, c.idgen, n.info) + if canOpenSym(result.sym): + if genericsOpenSym in c.features: + result = newOpenSym(result) + else: + result.flags.incl nfDisabledOpenSym + result.typ = nil else: result = n onUse(n.info, s) diff --git a/compiler/semtypes.nim b/compiler/semtypes.nim index 4f2d397a64fbd..bdf8fb8ca686b 100644 --- a/compiler/semtypes.nim +++ b/compiler/semtypes.nim @@ -1685,6 +1685,10 @@ proc semTypeExpr(c: PContext, n: PNode; prev: PType): PType = # unnecessary new type creation let alias = maybeAliasType(c, result, prev) if alias != nil: result = alias + elif n.typ.kind == tyFromExpr and c.inGenericContext > 0: + # sometimes not possible to distinguish type from value in generic body, + # for example `T.Foo`, so both are handled under `tyFromExpr` + result = n.typ else: localError(c.config, n.info, "expected type, but got: " & n.renderTree) result = errorType(c) @@ -2053,11 +2057,7 @@ proc semTypeNode(c: PContext, n: PNode, prev: PType): PType = elif op.s == "owned" and optOwnedRefs notin c.config.globalOptions and n.len == 2: result = semTypeExpr(c, n[1], prev) else: - if c.inGenericContext > 0 and n.kind == nkCall: - let n = semGenericStmt(c, n) - result = makeTypeFromExpr(c, n.copyTree) - else: - result = semTypeExpr(c, n, prev) + result = semTypeExpr(c, n, prev) of nkWhenStmt: var whenResult = semWhen(c, n, false) if whenResult.kind == nkStmtList: whenResult.transitionSonsKind(nkStmtListType) diff --git a/compiler/semtypinst.nim b/compiler/semtypinst.nim index 2764245125b7f..428d897ecf081 100644 --- a/compiler/semtypinst.nim +++ b/compiler/semtypinst.nim @@ -131,15 +131,61 @@ proc prepareNode(cl: var TReplTypeVars, n: PNode): PNode = replaceTypeVarsS(cl, n.sym, result.typ) else: replaceTypeVarsS(cl, n.sym, replaceTypeVarsT(cl, n.sym.typ)) - let isCall = result.kind in nkCallKinds - # don't try to instantiate symchoice symbols, they can be - # generic procs which the compiler will think are uninstantiated - # because their type will contain uninstantiated params - let isSymChoice = result.kind in nkSymChoices - for i in 0.. 1 and + (n[1].typ != nil and n[1].typ.kind == tyTypeDesc) + if ignoreFirst: + result.add(n[0]) + else: + result.add(prepareNode(cl, n[0])) + if n.len > 1: + if ignoreSecond: + result.add(n[1]) + else: + result.add(prepareNode(cl, n[1])) + for i in 2..= 2 + result.add(prepareNode(cl, n[0])) + result.add(n[1]) + for i in 2.. 0 and aOrig.n == nil and not c.isNoCall: + # don't match unresolved static value to static param to avoid + # faulty instantiations in calls in generic bodies + # but not for generic invocations as they only check constraints + result = isNone + elif f.base.kind notin {tyNone, tyGenericParam}: result = typeRel(c, f.base, a, flags) if result != isNone and f.n != nil: if not exprStructuralEquivalent(f.n, aOrig.n): @@ -1924,8 +1929,7 @@ proc typeRel(c: var TCandidate, f, aOrig: PType, # proc foo(T: typedesc, x: T) # when `f` is an unresolved typedesc, `a` could be any # type, so we should not perform this check earlier - if c.c.inGenericContext > 0 and - a.skipTypes({tyTypeDesc}).kind == tyGenericParam: + if c.c.inGenericContext > 0 and a.containsGenericType: # generic type bodies can sometimes compile call expressions # prevent unresolved generic parameters from being passed to procs as # typedesc parameters diff --git a/tests/generics/t23854.nim b/tests/generics/t23854.nim new file mode 100644 index 0000000000000..675f9d121a6d7 --- /dev/null +++ b/tests/generics/t23854.nim @@ -0,0 +1,70 @@ +# issue #23854, not entirely fixed + +import std/bitops + +const WordBitWidth = sizeof(pointer) * 8 + +func wordsRequired*(bits: int): int {.inline.} = + const divShiftor = fastLog2(uint32(WordBitWidth)) + result = (bits + WordBitWidth - 1) shr divShiftor + +type + Algebra* = enum + BLS12_381 + + BigInt*[bits: static int] = object + limbs*: array[wordsRequired(bits), uint] + + Fr*[Name: static Algebra] = object + residue_form*: BigInt[255] + + Fp*[Name: static Algebra] = object + residue_form*: BigInt[381] + + FF*[Name: static Algebra] = Fp[Name] or Fr[Name] + +template getBigInt*[Name: static Algebra](T: type FF[Name]): untyped = + ## Get the underlying BigInt type. + typeof(default(T).residue_form) + +type + EC_ShortW_Aff*[F] = object + ## Elliptic curve point for a curve in Short Weierstrass form + ## y² = x³ + a x + b + ## + ## over a field F + x*, y*: F + +type FieldKind* = enum + kBaseField + kScalarField + +func bits*[Name: static Algebra](T: type FF[Name]): static int = + T.getBigInt().bits + +template getScalarField*(EC: type EC_ShortW_Aff): untyped = + Fr[EC.F.Name] + +# ------------------------------------------------------------------------------ + +type + ECFFT_Descriptor*[EC] = object + ## Metadata for FFT on Elliptic Curve + order*: int + rootsOfUnity1*: ptr UncheckedArray[BigInt[EC.getScalarField().bits()]] # Error: in expression 'EC.getScalarField()': identifier expected, but found 'EC.getScalarField' + rootsOfUnity2*: ptr UncheckedArray[BigInt[getScalarField(EC).bits()]] # Compiler SIGSEGV: Illegal Storage Access + +func new*(T: type ECFFT_Descriptor): T = + discard + +# ------------------------------------------------------------------------------ + +template getBits[bits: static int](x: ptr UncheckedArray[BigInt[bits]]): int = bits + +proc main() = + let ctx = ECFFT_Descriptor[EC_ShortW_Aff[Fp[BLS12_381]]].new() + when false: echo getBits(ctx.rootsOfUnity2) # doesn't work yet? + doAssert ctx.rootsOfUnity1[0].limbs.len == wordsRequired(255) + doAssert ctx.rootsOfUnity2[0].limbs.len == wordsRequired(255) + +main() diff --git a/tests/generics/t23855.nim b/tests/generics/t23855.nim new file mode 100644 index 0000000000000..5ae927a94eb3f --- /dev/null +++ b/tests/generics/t23855.nim @@ -0,0 +1,61 @@ +# issue #23855, not entirely fixed + +import std/bitops + +const WordBitWidth = sizeof(pointer) * 8 + +func wordsRequired*(bits: int): int {.inline.} = + const divShiftor = fastLog2(uint32(WordBitWidth)) + result = (bits + WordBitWidth - 1) shr divShiftor + +type + Algebra* = enum + BLS12_381 + + BigInt*[bits: static int] = object + limbs*: array[wordsRequired(bits), uint] + + Fr*[Name: static Algebra] = object + residue_form*: BigInt[255] + + Fp*[Name: static Algebra] = object + residue_form*: BigInt[381] + + FF*[Name: static Algebra] = Fp[Name] or Fr[Name] + +template getBigInt*[Name: static Algebra](T: type FF[Name]): untyped = + ## Get the underlying BigInt type. + typeof(default(T).residue_form) + +type + EC_ShortW_Aff*[F] = object + ## Elliptic curve point for a curve in Short Weierstrass form + ## y² = x³ + a x + b + ## + ## over a field F + x*, y*: F + +func bits*[Name: static Algebra](T: type FF[Name]): static int = + T.getBigInt().bits + +# ------------------------------------------------------------------------------ + +type + ECFFT_Descriptor*[EC] = object + ## Metadata for FFT on Elliptic Curve + order*: int + rootsOfUnity*: ptr UncheckedArray[BigInt[Fr[EC.F.Name].bits()]] # Undeclared identifier `Name` + +func new*(T: type ECFFT_Descriptor): T = + discard + +# ------------------------------------------------------------------------------ + +template getBits[bits: static int](x: ptr UncheckedArray[BigInt[bits]]): int = bits + +proc main() = + let ctx = ECFFT_Descriptor[EC_ShortW_Aff[Fp[BLS12_381]]].new() + when false: echo getBits(ctx.rootsOfUnity) # doesn't work yet? + doAssert ctx.rootsOfUnity[0].limbs.len == wordsRequired(255) + +main() diff --git a/tests/generics/tcalltype.nim b/tests/generics/tcalltype.nim new file mode 100644 index 0000000000000..cba691f77c47b --- /dev/null +++ b/tests/generics/tcalltype.nim @@ -0,0 +1,26 @@ +discard """ + joinable: false # breaks everything because of #23977 +""" + +# issue #23406 + +template helper(_: untyped): untyped = + int + +type # Each of them should always be `int`. + GenA[T] = helper int + GenB[T] = helper(int) + GenC[T] = helper helper(int) + +block: + template helper(_: untyped): untyped = + float + + type + A = GenA[int] + B = GenB[int] + C = GenC[int] + + assert A is int # OK. + assert B is int # Fails; it is `float`! + assert C is int # OK. diff --git a/tests/generics/tuninstantiatedgenericcalls.nim b/tests/generics/tuninstantiatedgenericcalls.nim index 2163789e7bb25..172a00bd44163 100644 --- a/tests/generics/tuninstantiatedgenericcalls.nim +++ b/tests/generics/tuninstantiatedgenericcalls.nim @@ -163,3 +163,79 @@ block: # issue #23339 outerField: Inner[O.aToB] var x: Outer[A] doAssert typeof(x.outerField.innerField) is B + +block: # deref syntax + type + Enqueueable = concept x + x is ptr + Foo[T: Enqueueable] = object + x: typeof(default(T)[]) + + proc p[T](f: Foo[T]) = + var bar: Foo[T] + discard + var foo: Foo[ptr int] + p(foo) + doAssert foo.x is int + foo.x = 123 + doAssert foo.x == 123 + inc foo.x + doAssert foo.x == 124 + +block: + type Generic[T] = object + field: T + macro foo(x: typed): untyped = x + macro bar[T](x: typedesc[Generic[T]]): untyped = x + type + Foo[T] = object + field: Generic[int].foo() + Foo2[T] = object + field: Generic[T].foo() + Bar[T] = object + field: Generic[int].bar() + Bar2[T] = object + field: Generic[T].bar() + var x: Foo[int] + var x2: Foo2[int] + var y: Bar[int] + var y2: Bar2[int] + +block: + macro pick(x: static int): untyped = + if x < 100: + result = bindSym"int" + else: + result = bindSym"float" + + type Foo[T: static int] = object + fixed1: pick(25) + fixed2: pick(125) + unknown: pick(T) + + var a: Foo[123] + doAssert a.fixed1 is int + doAssert a.fixed2 is float + doAssert a.unknown is float + var b: Foo[23] + doAssert b.fixed1 is int + doAssert b.fixed2 is float + doAssert b.unknown is int + +import std/sequtils + +block: # version of #23432 with `typed`, don't delay instantiation + type + Future[T] = object + InternalRaisesFuture[T, E] = object + macro Raising[T](F: typedesc[Future[T]], E: varargs[typed]): untyped = + let raises = nnkTupleConstr.newTree(E.mapIt(it)) + nnkBracketExpr.newTree( + ident "InternalRaisesFuture", + nnkDotExpr.newTree(F, ident"T"), + raises + ) + type X[E] = Future[void].Raising(E) + proc f(x: X) = discard + var v: Future[void].Raising([ValueError]) + f(v) diff --git a/tests/metatype/ttypedescnotnimnode.nim b/tests/metatype/ttypedescnotnimnode.nim new file mode 100644 index 0000000000000..52a04815bd967 --- /dev/null +++ b/tests/metatype/ttypedescnotnimnode.nim @@ -0,0 +1,21 @@ +discard """ + errormsg: "type mismatch: got but expected 'typedesc'" + line: 14 +""" + +import macros + +# This is the same example as ttypeselectors but using a proc instead of a macro +# Instead of type mismatch for macro, proc just failed with internal error: getTypeDescAux(tyNone) +# https://github.com/nim-lang/Nim/issues/7231 + +proc getBase2*(bits: static[int]): typedesc = + if bits == 128: + result = newTree(nnkBracketExpr, ident("MpUintBase"), ident("uint64")) + else: + result = newTree(nnkBracketExpr, ident("MpUintBase"), ident("uint32")) + +type + MpUint2*[bits: static[int]] = getbase2(bits) +# technically shouldn't error until instantiation, so instantiate it +var x: MpUint2[123] diff --git a/tests/metatype/ttypeselectors.nim b/tests/metatype/ttypeselectors.nim index 5385a1be94919..d0843511d2181 100644 --- a/tests/metatype/ttypeselectors.nim +++ b/tests/metatype/ttypeselectors.nim @@ -98,16 +98,3 @@ var c: Bar[32] echo sizeof(a) echo sizeof(b) echo sizeof(c) - -# This is the same example but using a proc instead of a macro -# Instead of type mismatch for macro, proc just failed with internal error: getTypeDescAux(tyNone) -# https://github.com/nim-lang/Nim/issues/7231 - -proc getBase2*(bits: static[int]): typedesc = - if bits == 128: - result = newTree(nnkBracketExpr, ident("MpUintBase"), ident("uint64")) - else: - result = newTree(nnkBracketExpr, ident("MpUintBase"), ident("uint32")) - -type - MpUint2*[bits: static[int]] = getbase2(bits)