Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ICE on explicit generic proc with dependent-typed argument #7008

Closed
RedBeard0531 opened this issue Jan 1, 2018 · 5 comments · Fixed by #24005
Closed

ICE on explicit generic proc with dependent-typed argument #7008

RedBeard0531 opened this issue Jan 1, 2018 · 5 comments · Fixed by #24005

Comments

@RedBeard0531
Copy link
Contributor

type Node[T] = object
    val: T

# Compiles fine
proc concreteProc(s: Node[cstring]; key: s.T) = discard

# Also fine
proc implicitGenericProc1(s: Node; key: s.T) = discard

# still fine
proc explicitGenericProc1[T](s: Node[T]; key: T) = discard
    
# Internal Compiler Error!
proc explicitGenericProc2[T](s: Node[T]; key: s.T) = discard
Error: internal error: cannot resolve type: Node[explicitGenericProc2.T]

Repros on 0.17.2 and devel

@genotrance
Copy link
Contributor

AssertionError now with #head. Full stack trace:

Hint: used config file 'c:\users\gt\Desktop\DL\programming\nimdevel\config\nim.cfg' [Conf]
Hint: system [Processing]
Hint: temp [Processing]
nim.nim(133)             nim
nim.nim(97)              handleCmdLine
main.nim(162)            mainCommand
main.nim(73)             commandCompileToC
modules.nim(124)         compileProject
modules.nim(71)          compileModule
passes.nim(194)          processModule
passes.nim(103)          processTopLevelStmt
sem.nim(592)             myProcess
sem.nim(560)             semStmtAndGenerateGenerics
semstmts.nim(1893)       semStmt
semexprs.nim(910)        semExprNoType
semexprs.nim(2539)       semExpr
semstmts.nim(1684)       semProc
semstmts.nim(1510)       semProcAux
semstmts.nim(1121)       semParamList
semtypes.nim(1036)       semProcTypeNode
semtypes.nim(995)        semParamType
semtypes.nim(1529)       semTypeNode
semexprs.nim(2383)       semExpr
semexprs.nim(2420)       semExpr
semexprs.nim(231)        semConv
semcall.nim(379)         inferWithMetatype
sigmatch.nim(2048)       paramTypesMatch
sigmatch.nim(337)        paramTypesMatchAux
sigmatch.nim(1643)       typeRelImpl
sigmatch.nim(366)        concreteType
system.nim(3817)         raiseAssert
system.nim(2871)         sysFatal
Error: unhandled exception: false sigmatch.nim(366) cannot resolve type: Node[explicitGenericProc2.T] [AssertionError]

@narimiran
Copy link
Member

It works for me on Nim devel v1.3.5.

@RedBeard0531, can you confirm?

@ghost
Copy link

ghost commented Jul 9, 2020

@narimiran this fails:

type Node[T] = object
    val: T

# Compiles fine
proc concreteProc(s: Node[cstring]; key: s.T) = discard

# Also fine
proc implicitGenericProc1(s: Node; key: s.T) = discard

# still fine
proc explicitGenericProc1[T](s: Node[T]; key: T) = discard
    
# Internal Compiler Error!
proc explicitGenericProc2[T](s: Node[T]; key: s.T) = discard

let n = Node[int](val: 5)
implicitGenericProc1(n, 5) # works
explicitGenericProc1(n, 5) # works
explicitGenericProc2(n, 5) # doesn't

@narimiran
Copy link
Member

narimiran commented Jan 20, 2021

@narimiran this fails:
# Internal Compiler Error!

With Nim devel 1.5.1, explicitGenericProc2(n, 5) produces Error: cannot evaluate at compile time: s, which to me looks like a good (informative) error message.

@Clyybber
Copy link
Contributor

Clyybber commented Jan 21, 2021

This should work though, reopening :)

@Clyybber Clyybber reopened this Jan 21, 2021
@metagn metagn self-assigned this Aug 19, 2024
@Araq Araq closed this as completed in 69ea133 Aug 26, 2024
narimiran pushed a commit that referenced this issue Dec 12, 2024
…n fixes (#24005)

fixes #4228, fixes #4990, fixes #7006, fixes #7008, fixes #8406, fixes
(remaining issue fixed), refs #8545 (works properly now with
`cast[static[bool]]` changed to `cast[bool]`), refs #22342 and #22607
(disabled tests added), succeeds #23194

Parameter and return type nodes in generic procs now undergo the same
`inGenericContext` treatment that nodes in generic type bodies do. This
allows many of the fixes in #22029 and followups to also apply to
generic proc signatures. Like #23983 however this needs some more
compiler fixes, but this time mostly in `sigmatch` and type
instantiations.

1. `tryReadingGenericParam` no longer treats `tyCompositeTypeClass` like
a concrete type anymore, so expressions like `Foo.T` where `Foo` is a
generic type don't look for a parameter of `Foo` in non-generic code
anymore. It also doesn't generate `tyFromExpr` in non-generic code for
any generic LHS. This is to handle a very specific case in `asyncmacro`
which used `FutureVar.astToStr` where `FutureVar` is generic.
2. The `tryResolvingStaticExpr` call when matching `tyFromExpr` in
sigmatch now doesn't consider call nodes in general unresolved, only
nodes with `tyFromExpr` type, which is emitted on unresolved expressions
by increasing `c.inGenericContext`. `c.inGenericContext == 0` is also
now required to attempt instantiating `tyFromExpr`. So matching against
`tyFromExpr` in proc signatures works in general now, but I'm
speculating it depends on constant folding in `semExpr` for statics to
match against it properly.
3. `paramTypesMatch` now doesn't try to change nodes with `tyFromExpr`
type into `tyStatic` type when fitting to a static type, because it
doesn't need to, they'll be handled the same way (this was a workaround
in place of the static type instantiation changes, only one of the
fields in the #22647 test doesn't work with it).
4. `tyStatic` matching now uses `inferStaticParam` instead of just range
type matching, so `Foo[N div 2]` can infer `N` in the same way `array[N
div 2, int]` can. `inferStaticParam` also disabled itself if the
inferred static param type already had a node, but `makeStaticExpr`
generates static types with unresolved nodes, so we only disable it if
it also doesn't have a binding. This might not work very well but the
static type instantiation changes should really lower the amount of
cases where it's encountered.
5. Static types now undergo type instantiation. Previously the branch
for `tyStatic` in `semtypinst` was a no-op, now it acts similarly to
instantiating any other type with the following differences:
- Other types only need instantiation if `containsGenericType` is true,
static types also get instantiated if their value node isn't a literal
node. Ideally any value node that is "already evaluated" should be
ignored, but I'm not sure of a better way to check this, maybe if
`evalConstExpr` emitted a flag. This is purely for optimization though.
- After instantiation, `semConstExpr` is called on the value node if
`not cl.allowMetaTypes` and the type isn't literally a `static` type.
Then the type of the node is set to the base type of the static type to
deal with `semConstExpr` stripping abstract types.
We need to do this because calls like `foo(N)` where `N` is `static int`
and `foo`'s first parameter is just `int` do not generate `tyFromExpr`,
they are fully typed and so `makeStaticExpr` is called on them, giving a
static type with an unresolved node.

(cherry picked from commit 69ea133)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment