Nim
Nim copied to clipboard
nim-lang
do not match generic invocations to their base body types
fixes #24688, refs #5632
This would make sense if generic body types as the type of expressions are purely symbolic and never valid for an actual generic expression, which should produce some invocation type instead (liftParamType produces a generic invocation with args as tyAnything or the param constraints). Not sure if there is a valid counter case to this.
#5632 is a counter case which no longer works, but it doesn't really seem like a valid case, unless we still want to support it.
PR to arraymancer: https://github.com/mratsim/Arraymancer/pull/674, doesn't mean I'm sure this is correct though
try this as the if statement's body:
elif x.kind == tyGenericBody and f[0] == x:
result = isGeneric
for i in 1 ..< len(f):
if not f[i].acceptsAllTypes:
# not specific enough
result = isNone
break
with this defined in types.nim:
proc acceptsAllTypes*(t: PType): bool =
result = false
if t.kind == tyAnything:
result = true
elif t.kind == tyGenericParam:
if tfImplicitTypeParam in t.flags:
result = true
if not(t.hasElementType) or t.elementType.kind == tyNone:
result = true
actually this is similar to your first commit but there is no CI output. what happened?
I opened the PR after the second commit, also CI is fine outside of that arraymancer PR (arraymancer CI is broken) there was just a lot of httpclient failures. The first commit didn't work for any "working" cases either.
A problem with matching Foo[T] etc is that T cannot be inferred, and the case where it already is inferred should fail since it's a specific type. The tyCompositeTypeClass case should probably directly match itself but I don't know if it's needed. Most of the use cases here are to match the type Foo symbolically and are contained in typedesc which does not transform to tyCompositeTypeClass.
Ok that makes sense. So in the case of #5632 that should actually fail because it's attempting to match Option[T] with Option[int]. I didn't notice that. I guess if it were to work it would have to be a kind of inference, but I don't know how that could work exactly. Not that I want it to work anyway. Thanks for explaining.
Was waiting on https://github.com/mratsim/Arraymancer/pull/674 to be merged, testing to see if it actually works since arraymancer's own CI is broken
It does work but we should probably still wait for it to get merged as it would be bad to break arraymancer for this.
unless we still want to support it.
hm, unless there's a strong reason not to, it does look .. convenient to support - ie the resolution can be inferred based on the information present therefore it should be (or rather, why not?)
