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

SIGSEGV on generic macro returning a type #8406

Closed
Quelklef opened this issue Jul 23, 2018 · 7 comments · Fixed by #24005
Closed

SIGSEGV on generic macro returning a type #8406

Quelklef opened this issue Jul 23, 2018 · 7 comments · Fixed by #24005
Labels

Comments

@Quelklef
Copy link
Contributor

The following SIGSEGVs:

macro f(x: static[int]): untyped = discard
proc g[X: static[int]](v: f(X)) = discard

This is valid with the type system, though, right? Since X is known at compile-time it can be used to generate a type for v? I want to do something like this:

type MyFancyConcreteType = ...
type MyFancyGenericType[N: static[int]] = ...

macro Choose(N: static[int]): untyped =
  if N == 2:
    return ident("MyFancyConcreteType")
  else:
    return nnkBracketExpr.newTree(ident("MyFancyGenericType"), newIntLitNode(N))

var x: Choose(2)  # of MyFancyConcreteType
var y: Choose(3)  # of MyFancyGenericType[3]

# Generic procs may still be written
proc doSomethingWithEither[N: static[int]](v: Choose(N)) = ...

Is this possible, SIGSEGV or no; should I abandon the idea?

@Quelklef
Copy link
Contributor Author

I should also not that this is a static[T] issue; the following compiles fine (but does not "work"):

import macros

macro f(x: untyped): untyped =
  newIdentNode("int")

proc g[X](x: f(X)) =
  discard

@mratsim
Copy link
Collaborator

mratsim commented Jul 23, 2018

I don't know if it should work or not.

At the moment you can generate the proc in the macro or use a wrapper type like I did in Stint.

In you example that would be something like this:

macro Choose(N: static[int]): untyped =
  if N == 2:
    return ident("MyFancyConcreteType")
  else:
    return nnkBracketExpr.newTree(ident("MyFancyGenericType"), newIntLitNode(N))

type MyFancyConcreteType = ...
type MyFancyGenericType[N: static[int]] = ...

type MyFancyType[N: static[int]] = object
  impl: Choose[N]

var x: Choose(2)  # of MyFancyConcreteType
var y: Choose(3)  # of MyFancyGenericType[3]

# Generic procs may still be written
proc doSomethingWithEither[N: static[int]](v: MyFancyType[N]) = ...

You should put a full working example, btw. I think your issue is linked to #7231

@Quelklef
Copy link
Contributor Author

Yes, I believe you helped me implement your solution a while back. I decided I wanted to see if I could find a solution involving no wrapper types.

What do you mean by a full working example? In regards to the bug, or to my intended feature?

@mratsim
Copy link
Collaborator

mratsim commented Jul 24, 2018

I meant:

type MyFancyConcreteType = object
type MyFancyGenericType[N: static[int]] = object
  foo: array[N, int]

Instead of type MyFancyConcreteType = ... so that we can directly copy-paste like the lazy programmers we are :).

@Quelklef
Copy link
Contributor Author

Ah, I'll be sure to keep that in mind for the future ;-)

@saem
Copy link
Contributor

saem commented Mar 24, 2021

Tested in playground, no more sigsev, an error is produced instead: /usercode/in.nim(2, 28) Error: expression '' has no type (or is ambiguous)

@metagn
Copy link
Collaborator

metagn commented Sep 1, 2023

Early generic evaluation bug again, can link #8551 and #22607 right now but there are more related issues

As described in the other issues, delegating to a generic type is a workaround

macro f(x: static[int]): untyped = discard
type FWrapper[x: static int] = f(x)
proc g[X: static[int]](v: FWrapper[X]) = discard

@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
Labels
Projects
None yet
5 participants