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

Strange seq type error insanity #7321

Closed
StasRuinsEverything opened this issue Mar 10, 2018 · 14 comments
Closed

Strange seq type error insanity #7321

StasRuinsEverything opened this issue Mar 10, 2018 · 14 comments

Comments

@StasRuinsEverything
Copy link

Is this a bug?

type
    Shape = ref object of RootObj
    Box = ref object of Shape
    Circle = ref object of Shape

var x = Box()
var y = Circle()
var s: seq[Shape] = @[x] # Error: type mismatch: got <seq[Box]> but expected 'seq[Shape]'

If you replace the last line with the following, it starts working:

var s: seq[Shape] = @[x, y]
@Araq
Copy link
Member

Araq commented Mar 10, 2018

Not a bug. For @[x, y] the inferred type is seq[Shape] because that's the common base class for x and y. For @[x] the type is seq[Box], it's not converted to the base class because there is no need. Later the compiler figures it's incompatible with seq[Shape] (the subtype relation is not co-variant for seq) but by then it's too late. Workaround, type @[Shape(x)].

@Araq Araq closed this as completed Mar 10, 2018
@StasRuinsEverything
Copy link
Author

StasRuinsEverything commented Mar 11, 2018

This really makes no sense when you explicitly spell out what the type of seq[Shape] should be. By this sort of logic, passing a Box to a function accepting a Shape should give a type-error because the inferred type is Box and there's "no need to" convert to the base class, despite it being the only correct thing to do. Being consistent with a conceptually flawed algorithm doesn't exempt something from being a bug, if you ask me.

@Araq
Copy link
Member

Araq commented Mar 11, 2018

Being consistent with a conceptually flawed algorithm doesn't exempt something from being a bug, if you ask me.

The only thing here that is conceptually flawed is the subtyping relation itself since it doesn't compose easily. Co-/contravariance is a complex topic that depends on mutability aspects.

@StasRuinsEverything
Copy link
Author

StasRuinsEverything commented Mar 11, 2018

There can be no subtyping relation at all between the seqs; otherwise you could refer to a seq[Box] through a seq[Shape] and put a Circle in it. The flaw here is the failure of the compiler to abide by the request that the variable should be of type seq[Shape], and to try to cast the Box into a Shape. This doesn't imply that a seq[Box] is a subtype of a seq[Shape]. I think the compiler should perform your suggested workaround on its own.

@Araq
Copy link
Member

Araq commented Mar 12, 2018

The flaw here is the failure of the compiler to abide by the request that the variable should be of type seq[Shape], and to try to cast the Box into a Shape.

That's not how type checking works (?), @[x] is of type seq[Box] and every expression in Nim has a single type.

I think the compiler should perform your suggested workaround on its own.

Based on guesswork or what?

@StasRuinsEverything
Copy link
Author

StasRuinsEverything commented Mar 13, 2018

That's not how type checking works

This is not really a type-checking issue. It's an issue with the way types are assigned to literals in Nim. I'm not proposing that expressions should have more than one type; just that when there are multiple sane possibilities for what that single type could be, the type specifically requested by the user should be preferred if it's one of those possibilities. Maybe this isn't viable in the specific context of Nim for some reason that I'm not aware of, but it's definitely viable and reasonable in general.

Based on guesswork or what?

Based on the fact that the user did specify a type that can be unambiguously associated with the literal in question, and it's a perfectly good one.

@StasRuinsEverything
Copy link
Author

StasRuinsEverything commented Mar 13, 2018

This is perfectly valid Java:

public class Test {
    private static class Shape {}

    private static class Box extends Shape {}
    
    private static void test() {
        Shape[] s = {new Box()};
    }
}

This is perfectly valid C++:

class Shape {};

class Box: public Shape {};

int main() {
  Shape *s[] = {new Box()};
  return 0;
}

Clearly it is a problem with the way Nim does things, then, rather than some fundamental type theory issue about subtyping or the number of types an expression can have.

@Araq
Copy link
Member

Araq commented Mar 13, 2018

Well, ok, so you propose that sequence constructors have a different type than sequences. Been there, done that, creates much complexity, especially if you want to make seq a pure library type. See C++.

@Araq
Copy link
Member

Araq commented Mar 13, 2018

Clearly it is a problem with the way Nim does things, then, rather than some fundamental type theory issue about subtyping or the number of types an expression can have.

Certainly. I don't introduce type system complexity to support niche syntactic sugar for a paradigm that is hardly used in Nim (classical OO with "shapes").

@StasRuinsEverything
Copy link
Author

StasRuinsEverything commented Mar 13, 2018

so you propose that sequence constructors have a different type than sequences

Nope. I'm proposing that the algorithm which assigns types to literals should respect the surrounding context when it comes to such trivial things.

See C++.
I don't introduce type system complexity ...

Is the Java 5 type system also too complex?

... to support niche syntactic sugar for a paradigm that is hardly used in Nim (classical OO with "shapes").

It's not about "supporting syntax sugar". It's about not giving baffling and unnecessary errors, and abiding by the principle of least astonishment. If anything, Nim's behavior in this case is comparable to the many quirks of C++. In any case, you just keep moving the goalpost every time, so it's clear that you're unwilling to address this matter and I will press it no further. Nevertheless, I leave you with a riddle:

proc `@`[N, T](arr: array[N, T]): seq[T]
let x = Box()
let s: seq[Shape] = `@`[Mystery](array[1, Mystery](Mystery(x)))
Mystery = ?

@Araq
Copy link
Member

Araq commented Mar 13, 2018

Is the Java 5 type system also too complex?

Yes.

Nope. I'm proposing that the algorithm which assigns types to literals should respect the surrounding context when it comes to such trivial things.

It's about not giving baffling and unnecessary errors, and abiding by the principle of least astonishment.

Consider this:

var x: seq[Supertype] = @[subtype] # works in your proposal
overloadedProc(@[subtype]) # fails with your new proposal

"least astonishment", huh?

In any case, you just keep moving the goalpost every time, so it's clear that you're unwilling to address this matter and I will press it no further.

I'm not moving the goalpost every time, it's just that you don't really understand the involved tradeoffs and I do.

@StasRuinsEverything
Copy link
Author

StasRuinsEverything commented Mar 13, 2018

The second one should work as well.

you don't really understand the involved tradeoffs

Well, you're not doing a particularly good job elucidating them. It's not that you owe anyone a justification for your design decisions, but I don't understand why you bother at all if you're unwilling to address the core of the matter, which is evidently not about subtyping, or about it making Nim as complex as C++, or about it forcing sequence constructors to have different types from sequences. On the other hand, maybe I'm just doing a poor job explaining what I think should happen, which is really just some basic unification where most of the blanks are already filled in by the user-provided type annotations. As with the snippet that ends my previous comment, the type argument for the seq constructor should be decided by the type signature of the most specific compatible overloadedProc.

@Araq
Copy link
Member

Araq commented Mar 14, 2018

type
  Shape = ref object of RootObj
  ShapeB = ref object of Shape
  Box = ref object of ShapeB
  Circle = ref object of ShapeB

var x = Box()
var y = Circle()
var s: seq[Shape] = @[x, y]

has the same issue fwiw. The compiler is consistent in its stupidity.

@zah
Copy link
Member

zah commented Mar 15, 2018

FWIW, what the OP is asking for here may be covered by the planned return-type overloading. The request is that the type annotation on the left-hand side of the assignment should affect how the type of the expression on the right-hand side is resolved.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants