-
-
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
always reinstantiate nominal values of generic instantiations #24425
Conversation
Hits a pre-existing bug: type
Foo[T] = object of RootObj
x: T
Bar = object of Foo[int]
proc foo(x: Foo) = discard
foo(Bar()) This works if Same for type Foo[T] = object
x: T
proc main =
var f: Foo[int]
proc `=destroy`[T](f: var Foo[T]) =
# should say: cannot bind another '=destroy' to: Foo
discard This one is lower priority though IMO |
Will try to split the fixes for the above pre-existing issues as well as the one in |
split from #24425 The added test did not work previously. The result of `getTypeImpl` is the uninstantiated AST of the original type symbol, and the macro attempts to use this type for the result. To fix the issue, the provided `typedesc` argument is used instead.
split from #24425 Matching `tyGenericBody` performs a match on the last child of the generic body, in this case the uninstantied `tyObject` type. If the object contains no generic fields, this ends up being the same type as all instantiated ones, but if it does, a new type is created. This fails the `sameObjectTypes` check that subtype matching for object types uses. To fix this, also consider that the pattern type could be the generic uninstantiated object type of the matched type in subtype matching.
Did not move the fix for |
Thanks for your hard work on this PR! Hint: mm: orc; opt: speed; options: -d:release |
follows up #24425, fixes #18861, fixes #22445 Since #24425 generic object and distinct types now accurately link back to their generic instantiations. To work around this issue previously, type matching checked if generic objects/distinct types were *structurally* equal, which caused false positives with types that didn't use generic parameters in their structures. This structural check is now removed, in cases where generic objects/distinct types require a nontrivial equality check, the generic parameters of the `typeInst` fields are checked for equality instead. The check is copied from `tyGenericInst`, but the check in `tyGenericInst` is not always sufficient as this type can be skipped or unreachable in the case of `ref object`s.
fixes #22479, fixes #24374, depends on #24429 and #24430
When instantiating generic types which directly have nominal types (object, distinct, ref/ptr object but not enums1) as their values, the nominal type is now copied (in the case of ref objects, its child as well) so that it receives a fresh ID and
typeInst
field. Previously this only happened if it contained any generic types in its structure, as is the case for all other types.This solves #22479 and #24374 by virtue of the IDs being unique, which is what destructors check for. Technically types containing generic param fields work for the same reason. There is also the benefit that the
typeInst
field is correct. However issues like #22445 aren't solved because the compiler still uses structural object equality checks for inheritance etc. which could be removed in a later PR.Also fixes a pre-existing issue where destructors bound to object types with generic fields would not error when attempting to define a user destructor after the fact, but the error message doesn't show where the implicit destructor was created now since it was only created for another instance. To do this, a type flag is used that marks the generic type symbol when a generic instance has a destructor created. Reusing
tfCheckedForDestructor
for this doesn't work.Maybe there is a nicer design that isn't an overreliance on the ID mechanism, but the shortcomings of
tyGenericInst
are too ingrained in the compiler to use for this. I thought about maybe adding something liketyNominalGenericInst
, but it's really much easier if the nominal type itself directly contains the information of its generic parameters, or at least its "symbol", which the design is heading towards.Footnotes
See this test in enumutils. The field symbols
b0
/b1
always have the uninstantiated typeB
because enum fields don't expect to be generic, so no generic instance ofB
matches its own symbols. Wouldn't expect anyone to use generic enums but maybe someone does. ↩