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

Regression from 2.0 to version-2-2/devel in template overload resolution with generics #24146

Open
tersec opened this issue Sep 21, 2024 · 5 comments
Labels
Documentation Content Related to documentation content (not generation).

Comments

@tersec
Copy link
Contributor

tersec commented Sep 21, 2024

Description

template g(h: typedesc): untyped = {.error: "2.0 doesn't hit".}
type
  S[Y] = array[Y.g, int]
  N = object
template g(_: type N): untyped = 1
discard sizeof(S[N])

Nim Version

Builds:

Nim Compiler Version 2.0.8 [Linux: amd64]
Compiled at 2024-09-21
Copyright (c) 2006-2023 by Andreas Rumpf

git hash: 5935c3bfa9fec6505394867b23510eb5cbab3dbf
active boot switches: -d:release
Nim Compiler Version 2.0.9 [Linux: amd64]
Compiled at 2024-09-21
Copyright (c) 2006-2023 by Andreas Rumpf

git hash: 27381cc60213e19aa34664176bd358ca5e45bd5a
active boot switches: -d:release

Does not build:

Nim Compiler Version 2.1.99 [Linux: amd64]
Compiled at 2024-09-21
Copyright (c) 2006-2024 by Andreas Rumpf

git hash: 755307be61e4ee7b32c8354b2c303d04bdfc3a3e
active boot switches: -d:release
Nim Compiler Version 2.1.99 [Linux: amd64]
Compiled at 2024-09-21
Copyright (c) 2006-2024 by Andreas Rumpf

git hash: d51d88700b2fb3bd228d5e8f7385e2e4a2e2880c
active boot switches: -d:release

Current Output

/tmp/w.nim(3, 17) template/generic instantiation of `g` from here
/tmp/w.nim(1, 43) Error: 2.0 doesn't hit

Expected Output

No response

Known Workarounds

No response

Additional Information

No response

@metagn
Copy link
Collaborator

metagn commented Sep 21, 2024

Caused by #23983 because dotfields use semGenericStmt now which makes g an nkClosedSymChoice with 1 element where it was left as nkIdent before. This is also the behavior of generic procs, but mixin doesn't work here because the semGenericStmt is localized to the dot expression (maybe it should be done beforehand for the entire expression?). We could make these expressions always mixin, but this might break stuff too if people depend on the binding behavior.

This is the workaround:

template g(h: typedesc): untyped = {.error: "2.0 doesn't hit".}
template gMixin(h: untyped): untyped =
  mixin g
  g(h)
type
  S[Y] = array[Y.gMixin, int]
  N = object
template g(_: type N): untyped = 1
discard sizeof(S[N])

If mixin is default, then people would have to use this workaround for bind. We could also allow something like array[(mixin g; Y.g), int] and the same for bind but it would be uglier and wouldn't work on older versions.

So which is more reasonable, using the workaround, or mixin by default?

@arnetheduck
Copy link
Contributor

"sometimes mixin by default" is an entirely unteachable rule, so it's a matter of picking one semantic for when mixin is needed and roll with it - in that sense, it looks to me like @metagn's solution is not a workaround but rather "how it should work" - it's not that terrible even if it introduces some ceremony.

That said, mixin has historically been a difficult thing to teach / explain in general - it's one of those "sprinkle some mixin here, maybe it'll solve your problem" kind of constructs, and part of the reason it has gained such notoriety is indeed that it's been inconsistently implemented with lots of complementary bugs where the compiler would look up entirely random stuff at times, ie depending on import order or any number of things. Recent bugfixing in the area of templates, generics and lookups maybe will paint a clearer picture.

@metagn metagn added the Documentation Content Related to documentation content (not generation). label Sep 22, 2024
@arnetheduck
Copy link
Contributor

is there a case ever where bind is useful / required, if the mixin behavior is never the default?

@Araq
Copy link
Member

Araq commented Sep 23, 2024

But mixin is the default for every overloaded symbol in scope. A better rule might be like C++'s template lookup rules work but it looked worse on paper when I designed the feature, consider:

proc sortHelper[T](....) = ...

proc sort[T](a: var openArray[T]) = 
  sortHelper(a) # overridable as `a` depends on `T`?!

@metagn
Copy link
Collaborator

metagn commented Sep 23, 2024

It's the same resolution as generic procs. It has the bind behavior when a symbol has only 1 overload, and mixin otherwise (this is debated in #3542/#11184). But also mixin only accepts new global symbols during resolution (#14661).

I don't remember having to use bind in a generic proc except to workaround a Nim bug in an old version, "only use the overloads defined up to this point" isn't a common requirement for symbols that are already overloaded. A counter example would be system.`==` for pointer equality for ref types as opposed to custom == overloads, but at that point I would just use literally system.`==`.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Documentation Content Related to documentation content (not generation).
Projects
None yet
Development

No branches or pull requests

4 participants