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

type Bar = (int, seq, array, tuple, set) should give CT error #14800

Open
timotheecour opened this issue Jun 25, 2020 · 4 comments
Open

type Bar = (int, seq, array, tuple, set) should give CT error #14800

timotheecour opened this issue Jun 25, 2020 · 4 comments

Comments

@timotheecour
Copy link
Member

type Bar = (int, seq, array, tuple, set) should give CT error

(similar to recent bug fixes but this one is still not fixed)

Example

when true:
  type Bar = (int, seq, array, tuple, set) # should probably be disallowed because all elements after position 1 are not concrete types
  echo Bar.sizeof # Error: internal error: getTypeDescAux(tyGenericParam)

Current Output

Error: internal error: getTypeDescAux(tyGenericParam)

Expected Output

this should give CT error but is currently accepted

type Bar = (int, seq, array, tuple, set)

Additional Information

@timotheecour timotheecour added Invalid Code Acceptance Everything related to compiler not complaining about invalid code Compiler Crash labels Jun 25, 2020
@metagn
Copy link
Collaborator

metagn commented May 14, 2023

It should compile, it's a typeclass in theory. Though it currently crashes as one.

type Bar = (int, seq, array, tuple, set)

echo (int, seq[int], array[1, int], (int,), set[int8]) is Bar # invalid kind for lastOrd(tyGenericParam), but should be true

@metagn
Copy link
Collaborator

metagn commented May 19, 2023

The specific error here is caused by array, seq and set compile and tuple gives another manual error

template tupleTypeClass(base, concrete) =
  type Bar = tuple[a: base]
  echo tuple[a: concrete] is Bar

tupleTypeClass seq, seq[int] # compiles, true
tupleTypeClass set, set[int8] # compiles, true
tupleTypeClass array, array[2, int] # internal error: invalid kind for lastOrd(tyGenericParam)
tupleTypeClass tuple, (int, int) # Error: 'tuple' is not a concrete type

It's actually not recognized as a typeclass in the cases where it compiles with seq and set

type Bar = tuple[a: seq]

proc foo(x: Bar) =
  echo x # type mismatch
  # discard instead gives "invalid type T in this context proc (x: Bar) ..."
  # same if we just write tuple[a: seq] instead of Bar

foo((a: @[1, 2]))

So either we don't allow typeclass tuple types (and point this out in the documentation), or we lift these to typeclasses.

@Araq
Copy link
Member

Araq commented May 21, 2023

... or we lift these to typeclasses.

Seems better than a special case in the manual that reflects an oversight in the compiler.

@metagn metagn added Feature and removed Invalid Code Acceptance Everything related to compiler not complaining about invalid code Feature labels Jun 11, 2023
@metagn
Copy link
Collaborator

metagn commented Jun 11, 2023

Apparently in general concrete types lifting to typeclasses gives different results.

type Foo = array[2, tuple]

echo array[2, (int, int)] is Foo # true
echo array[2, int] is Foo # false
proc foo(x: Foo) = echo x
foo({1: 2, 3: 4}) # [(1, 2), (3, 4)]

var x: Foo # invalid type: ':anon:type' in this context: 'Foo' for var
type Foo = array[2, array]
# internal error: invalid kind for lastOrd(tyGenericParam)

The manual error for tuple is caused by an explicit check in semstmts for just objects and tuples:

Nim/compiler/semstmts.nim

Lines 1557 to 1558 in 21d941c

if s.typ.kind in {tyObject, tyTuple} and not s.typ.n.isNil:
checkForMetaFields(c, s.typ.n)

For objects it makes sense, they should never be typeclasses (same for any nominal type). But for other types this check could be used to mark the entire type as meta to error when we try to use it as a concrete type.

The manual says this:

A type class is a special pseudo-type that can be used to match against types in the context of overload resolution or the is operator.

Type classes can be combined using the standard boolean operators to form more complex type classes

Nothing about combining type classes with concrete types. So we don't really have an obligation to support this, but also the compiler happens to fundamentally support it in both use cases, overload resolution and the is operator.

In any case the compiler crash with array should be addressed.

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

No branches or pull requests

3 participants