-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Destructors + concepts semcheck issue #12620
Comments
Is this a complete example that I can compile somehow? |
It's been a while but I think so, you need to compile the first file. |
Araq
added a commit
that referenced
this issue
Oct 25, 2024
closes nim-lang/RFCs#380, fixes #4773, fixes #14729, fixes #16755, fixes #18150, fixes #22984, refs #11167 (only some comments fixed), refs #12620 (needs tiny workaround) The compiler gains a concept of root "nominal" types (i.e. objects, enums, distincts, direct `Foo = ref object`s, generic versions of all of these). Exported top-level routines in the same module as the nominal types that their parameter types derive from (i.e. with `var`/`sink`/`typedesc`/generic constraints) are considered attached to the respective type, as the RFC states. This happens for every argument regardless of placement. When a call is overloaded and overload matching starts, for all arguments in the call that already have a type, we add any operation with the same name in the scope of the root nominal type of each argument (if it exists) to the overload match. This also happens as arguments gradually get typed after every overload match. This restricts the considered overloads to ones attached to the given arguments, as well as preventing `untyped` arguments from being forcefully typed due to unrelated overloads. There are some caveats: * If no overloads with a name are in scope, type bound ops are not triggered, i.e. if `foo` is not declared, `foo(x)` will not consider a type bound op for `x`. * If overloads in scope do not have enough parameters up to the argument which needs its type bound op considered, then type bound ops are also not added. For example, if only `foo()` is in scope, `foo(x)` will not consider a type bound op for `x`. In the cases of "generic interfaces" like `hash`, `$`, `items` etc. this is not really a problem since any code using it will have at least one typed overload imported. For arbitrary versions of these though, as in the test case for #12620, a workaround is to declare a temporary "template" overload that never matches: ```nim # neither have to be exported, just needed for any use of `foo`: type Placeholder = object proc foo(_: Placeholder) = discard ``` I don't know what a "proper" version of this could be, maybe something to do with the new concepts. Possible directions: A limitation with the proposal is that parameters like `a: ref Foo` are not attached to any type, even if `Foo` is nominal. Fixing this for just `ptr`/`ref` would be a special case, parameters like `seq[Foo]` would still not be attached to `Foo`. We could also skip any *structural* type but this could produce more than one nominal type, i.e. `(Foo, Bar)` (not that this is hard to implement, it just might be unexpected). Converters do not use type bound ops, they still need to be in scope to implicitly convert. But maybe they could also participate in the nominal type consideration: if `Generic[T] = distinct T` has a converter to `T`, both `Generic` and `T` can be considered as nominal roots. The other restriction in the proposal, being in the same scope as the nominal type, could maybe be worked around by explicitly attaching to the type, i.e.: `proc foo(x: T) {.attach: T.}`, similar to class extensions in newer OOP languages. The given type `T` needs to be obtainable from the type of the given argument `x` however, i.e. something like `proc foo(x: ref T) {.attach: T.}` doesn't work to fix the `ref` issue since the compiler never obtains `T` from a given `ref T` argument. Edit: Since the module is queried now, this is likely not possible. --------- Co-authored-by: Andreas Rumpf <[email protected]>
narimiran
pushed a commit
that referenced
this issue
Jan 14, 2025
closes nim-lang/RFCs#380, fixes #4773, fixes #14729, fixes #16755, fixes #18150, fixes #22984, refs #11167 (only some comments fixed), refs #12620 (needs tiny workaround) The compiler gains a concept of root "nominal" types (i.e. objects, enums, distincts, direct `Foo = ref object`s, generic versions of all of these). Exported top-level routines in the same module as the nominal types that their parameter types derive from (i.e. with `var`/`sink`/`typedesc`/generic constraints) are considered attached to the respective type, as the RFC states. This happens for every argument regardless of placement. When a call is overloaded and overload matching starts, for all arguments in the call that already have a type, we add any operation with the same name in the scope of the root nominal type of each argument (if it exists) to the overload match. This also happens as arguments gradually get typed after every overload match. This restricts the considered overloads to ones attached to the given arguments, as well as preventing `untyped` arguments from being forcefully typed due to unrelated overloads. There are some caveats: * If no overloads with a name are in scope, type bound ops are not triggered, i.e. if `foo` is not declared, `foo(x)` will not consider a type bound op for `x`. * If overloads in scope do not have enough parameters up to the argument which needs its type bound op considered, then type bound ops are also not added. For example, if only `foo()` is in scope, `foo(x)` will not consider a type bound op for `x`. In the cases of "generic interfaces" like `hash`, `$`, `items` etc. this is not really a problem since any code using it will have at least one typed overload imported. For arbitrary versions of these though, as in the test case for #12620, a workaround is to declare a temporary "template" overload that never matches: ```nim # neither have to be exported, just needed for any use of `foo`: type Placeholder = object proc foo(_: Placeholder) = discard ``` I don't know what a "proper" version of this could be, maybe something to do with the new concepts. Possible directions: A limitation with the proposal is that parameters like `a: ref Foo` are not attached to any type, even if `Foo` is nominal. Fixing this for just `ptr`/`ref` would be a special case, parameters like `seq[Foo]` would still not be attached to `Foo`. We could also skip any *structural* type but this could produce more than one nominal type, i.e. `(Foo, Bar)` (not that this is hard to implement, it just might be unexpected). Converters do not use type bound ops, they still need to be in scope to implicitly convert. But maybe they could also participate in the nominal type consideration: if `Generic[T] = distinct T` has a converter to `T`, both `Generic` and `T` can be considered as nominal roots. The other restriction in the proposal, being in the same scope as the nominal type, could maybe be worked around by explicitly attaching to the type, i.e.: `proc foo(x: T) {.attach: T.}`, similar to class extensions in newer OOP languages. The given type `T` needs to be obtainable from the type of the given argument `x` however, i.e. something like `proc foo(x: ref T) {.attach: T.}` doesn't work to fix the `ref` issue since the compiler never obtains `T` from a given `ref T` argument. Edit: Since the module is queried now, this is likely not possible. --------- Co-authored-by: Andreas Rumpf <[email protected]> (cherry picked from commit 2864830)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
I get a
cannot instantiate =destroy
in the following complex case. The workaround is to have a dummy global variable of the type that has the destructor or that contains it.While it seems like a similar cross-module visibility issue that also involves mixin like the generic sandwich #11225, just exporting the
=destroy
that had the mixin doesn't solve the issue so it's probably something else.Tagging low priority as there are 4 workarounds:
File 1: runtime.nim
Compile this to reproduce the problem
File 2: context_thread_local.nim
2 fixes are possible in this file, exporting "tasks" or instantiating a magic variable
File 3: listdeques.nim
Remove the concept and everything works as well
File 4: tasks.nim
The base type, it just needs to implement the concept
The text was updated successfully, but these errors were encountered: