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

generics: error with multiple types and constraints #7794

Closed
brentp opened this Issue May 7, 2018 · 11 comments

Comments

Projects
None yet
6 participants
@brentp
Copy link
Contributor

brentp commented May 7, 2018

Update: @krux02 has found this happens when constraints are specified. When the constraints are removed the code will compile.

the code below fails with: Error: cannot instantiate Data got: <type int, type float64> but expected: <T: SomeNumber, U: SomeNumber>

type
  Element*[U:SomeNumber] = ref object
    value*: U

  Data*[T:SomeNumber, U:SomeNumber] = ref object
    xs*: seq[T]
    e*: Element[U]

var e = Element[float64](value:2'f64)
var xs = new_seq[int](10)
var d = Data[int, float64](xs:xs, e:e)
@narimiran

This comment has been minimized.

Copy link
Member

narimiran commented May 7, 2018

Even this doesn't work:

type
  Element*[U: float64] = ref object
    value*: U

  Data*[T: int, U: float64] = ref object
    xs*: seq[T]
    e*: Element[U]

let e = Element[float64](value:2'f64)
let xs = new_seq[int](10)
var d = Data[int, float64](xs: xs, e: e)
@brentp

This comment has been minimized.

Copy link
Contributor

brentp commented May 7, 2018

or this:

type
  Data*[T:SomeNumber, U:SomeReal] = ref object
    x*: T
    value*: U

var d = Data[int, float64](x:10.int, value:2'f64)

@brentp brentp changed the title generics: error with multiple types generics: error with multiple types and constraints May 7, 2018

@krux02

This comment has been minimized.

Copy link
Contributor

krux02 commented May 7, 2018

well as close as I can get between working and non working with non-essential parts removed.

type
  # this works
  DataA*[T, U] = object
    xs*: seq[T]
    e*: U

  # this doesn't
  DataB*[T:SomeNumber; U:SomeNumber] = object
    xs*: seq[T]
    e*: U

var a = DataA[int, float64](xs: @[1,2,3], e: 12.3)
var b = DataB[int, float64](xs: @[1,2,3], e: 12.3)
@LemonBoy

This comment has been minimized.

Copy link
Collaborator

LemonBoy commented Jul 8, 2018

There are two factors contributing to this:

  • In matchesAux, before paramTypesMatch we should really reset m.typedescMatched = false
  • The typeRel handling of tyOr (and tyAnd/tyNot I think) is fishy: when T:SomeNumber is run first trough typerel the bindingRet ties it to the type it matched (tyInt in the snippet above). This means that when it's time to process U:SomeNumber the tyOr is fetched from the cache as tyInt (WTF?) and then is compared to tyFloat64, yielding isNone and making the instantiation fail.

CC @zah

@zah

This comment has been minimized.

Copy link
Member

zah commented Jul 8, 2018

m.typedescMatched is a very hacky work-around trying to compensate for one very old historic flaw of the compiler internal design. The generic params "signature" of a type is still consisting of tyGenericParams slots as formal requirement, while it should really be using tyTypeDesc. The meaning of tyGenericParam is "match any value", while tyTypeDesc has the proper meaning of "match any type". Unfortunately, fixing this has proven to be quite hard in the few times I've attempted to solve the problem, so the compiler is still relying on these subtly wrong m.typedescMatched hacks.

Regarding your point 2, you seem to be missing the fact that each named type class is a "bind once" type by default. This is briefly explained in the type classes section of the manual:
https://nim-lang.org/docs/manual.html#generics-type-classes

In this sense, Brent's example is the best when it comes to solving this issue, because it doesn't invoke this extra complexity:

type
  Data*[T:SomeNumber, U:SomeReal] = ref object
    x*: T
    value*: U

var d = Data[int, float64](x:10.int, value:2'f64)

With that said, I'll mention for completeness that there are indeed problems with the or handling in the compiler. Properly handling or types in all situations requires back-tracking in the type binding process. We'll eventually introduce some support for this in the compiler to deal with some more easily triggered cases involving concepts.

@LemonBoy

This comment has been minimized.

Copy link
Collaborator

LemonBoy commented Jul 8, 2018

There's definitely more than it meets the eye.

very hacky work-around

Yeah, I noticed some comments about this in the compiler.

Regarding your point 2, you seem to be missing the fact that each named type class is a "bind once" type by default.

Oh, I've completely missed that bit.

In this sense, Brent's example is the best when trying to solve this issue, because it doesn't invoke this extra complexity:

Definitely

Resetting m.typedescMatched fixes the issue at hand, do you think it may have other unintended consequences?

@zah

This comment has been minimized.

Copy link
Member

zah commented Jul 8, 2018

Sounds like a plausible solution, but as usual the test suite will tell you if there are problems.

LemonBoy added a commit to LemonBoy/Nim that referenced this issue Jul 8, 2018

Reset typedescMatched before paramTypesMatch
The flag should not be carried out across different parameters.

Fixes nim-lang#7794
@Araq

This comment has been minimized.

Copy link
Member

Araq commented Jul 9, 2018

The generic params "signature" of a type is still consisting of tyGenericParams slots as formal requirement, while it should really be using tyTypeDesc.

That's not true, tyTypeDesc is just as flawed for this as typically you want to allow e.g. int but not typeDesc[int] in your generic.

LemonBoy added a commit to LemonBoy/Nim that referenced this issue Jul 9, 2018

Reset typedescMatched before paramTypesMatch
The flag should not be carried out across different parameters.

Fixes nim-lang#7794

@Araq Araq closed this in #8250 Jul 9, 2018

Araq added a commit that referenced this issue Jul 9, 2018

Reset typedescMatched before paramTypesMatch (#8250)
The flag should not be carried out across different parameters.

Fixes #7794
@zah

This comment has been minimized.

Copy link
Member

zah commented Jul 9, 2018

That's not true, tyTypeDesc is just as flawed for this as typically you want to allow e.g. int but not typeDesc[int] in your generic.

That's a problem only due to the heavy use of skipTypes(tyTypeDesc) within the compiler. If we want to support arbitrary "type of type of type" chains, we'll have to move on to skipping only a single level everywhere where this is appropriate and then the problem you mentioned will go away.

@Araq

This comment has been minimized.

Copy link
Member

Araq commented Jul 9, 2018

No, it's not an implementation problem, it's language design / spec problem: type C[T] means type C[T: typedesc and not (typedesc of typedesc)].

@zah

This comment has been minimized.

Copy link
Member

zah commented Jul 9, 2018

I don't disagree with this, but it doesn't contradict anything I said. [typedesc[int]] wouldn't match typedesc[typedesc[int]] in the future I'm describing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment