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

Using type instead of typedesc in template signature fails compilation #11058

Closed
kaushalmodi opened this issue Apr 18, 2019 · 25 comments

Comments

Projects
None yet
10 participants
@kaushalmodi
Copy link
Contributor

commented Apr 18, 2019

I was experimenting with a template that works like a generic proc, and @rayman22201 helped discover this issue where if I use type instead of typedesc and the passed type is a type alias, the compilation fails.

Example

# Issue: https://irclogs.nim-lang.org/18-04-2019.html#04:14:16
template arrayToObj(S: type, U: type, arr: untyped): untyped =            # doesn't work
# template arrayToObj(S: typedesc, U: typedesc, arr: untyped): untyped =  # works
  let
    inputSeqPtr = cast[ptr UncheckedArray[S]](unsafeAddr arr[0])
    sLen = arr.len
    inputSizeArray = [sLen]
    inputSizeArrayPtr = cast[ptr UncheckedArray[cint]](unsafeAddr inputSizeArray[0])
    retVal = cast[ptr U](alloc0(sizeof U))

  # for i in 0 ..< sLen:
  #   echo $inputSeqPtr[][i]

  retVal[].data = inputSeqPtr
  retVal[].size = inputSizeArrayPtr
  # pseudo return
  retVal

type
  boolean_T = cuchar
  emxArray_int32_T* = object
    data*: ptr UncheckedArray[cint]
    size*: ptr UncheckedArray[cint]
    allocatedSize*: cint
    numDimensions*: cint
    canFreeData*: boolean_T
  IntArrayObj* = emxArray_int32_T

let
  data = @[632.cint, 68, 87, 73, 22, 63, 20]
  dataObjPtr = arrayToObj(cint, IntArrayObj, data)

echo "data:"
for i in 0 ..< dataObjPtr[].size[][0]:
  echo "  ", $dataObjPtr[].data[][i]

Current Output

t.nim(31, 26) Error: type mismatch: got <type cint, type emxArray_int32_T, seq[cint]>
but expected one of:
template arrayToObj(S: type; U: type; arr: untyped): untyped
  first type mismatch at position: 2
  required type: type
  but expression 'IntArrayObj' is of type: type emxArray_int32_T

expression: arrayToObj(cint, IntArrayObj, data)

Expected Output

data:
  632
  68
  87
  73
  22
  63
  20

Possible Solution

Use typedesc instead of type in template signature where the passed argument is a type (type alias or not).

Additional Information

$ nim -v
Nim Compiler Version 0.19.9 [Linux: amd64]
Compiled at 2019-04-15
Copyright (c) 2006-2019 by Andreas Rumpf

git hash: e2848ccd2bb1da93d729a6dffb201cea80b9cae4
active boot switches: -d:release

Ref: https://irclogs.nim-lang.org/18-04-2019.html#04:14:16

@mratsim mratsim added the Semcheck label Apr 18, 2019

@mratsim

This comment has been minimized.

Copy link
Collaborator

commented Apr 18, 2019

I'm pretty sure we had another magic fix last week by using typedesc instead of type even though they are supposed to be the same but I can't find my question in the IRC logs cc @Araq.

@Vindaar

This comment has been minimized.

Copy link
Contributor

commented Apr 18, 2019

@mratsim just stumbled upon that 2 minutes ago. It was here:
https://forum.nim-lang.org/t/4775
and the fix:
4974f99

@rayman22201

This comment has been minimized.

Copy link
Contributor

commented Apr 18, 2019

If typedesc is preferred as @Araq says in the forum post, then manual section on templates has several incorrect examples:

@Araq

This comment has been minimized.

Copy link
Member

commented Apr 19, 2019

IMO the type keyword is for type sections, typedesc is the meta-type for types and typeof is for getting the type from an expression. Clarity of names leads to a clarity of thought that easily trumps any arguments about "consistency for all the type classes". In other words, different things should have different names, optimizing for the number of keywords is utter nonsense.

@kaushalmodi

This comment has been minimized.

Copy link
Contributor Author

commented Apr 19, 2019

type keyword is for type sections

You mean the keyword to define custom types, type aliases, etc. right?

typedesc is the meta-type for types

So everywhere in the documentation (like the https://nim-lang.github.io/Nim/manual.html#templates-identifier-construction @rayman22201 linked above), we should use typedesc instead of type in the template/macro signatures, right?

@rayman22201

This comment has been minimized.

Copy link
Contributor

commented Apr 19, 2019

@Araq, I think this is a documentation bug.

Clarity of names leads to a clarity of thought

I don't think @kaushalmodi or I am suggesting that the keywords are badly designed. I agree with you!

I think the examples and the docs are not clear about this. Those examples should use typedesc based on what you are saying, and there should be a paragraph in the docs that explain the difference between type and typedesc, because it's not obvious at first glance, and there are no examples that use typedesc in the manual.

@kaushalmodi

This comment has been minimized.

Copy link
Contributor Author

commented Apr 19, 2019

I don't think @kaushalmodi or I am suggesting that the keywords are badly designed. I agree with you!

Yes.

@rayman22201

This comment has been minimized.

Copy link
Contributor

commented Apr 19, 2019

ping @narimiran, master of docs :-P

@Araq

This comment has been minimized.

Copy link
Member

commented Apr 19, 2019

we should use typedesc instead of type in the template/macro signatures, right?

Right. It's what we used to use too.

@bluenote10

This comment has been minimized.

Copy link
Contributor

commented Apr 20, 2019

Should we go as far as deprecating type for arguments?

Note that issue #9731 suggests exactly the opposite ;).

In my opinion the distinction between type and typedesc added clarification. Also, typedesc will probably remain a keyword for a while, so a reduction of keywords is unlikely. In my opinion have two things refer to the same is the most confusing solution -- as seen multiple times already.

@Araq

This comment has been minimized.

Copy link
Member

commented Apr 20, 2019

Should we go as far as deprecating type for arguments?

For now let's document it consistently, but eventually we should get rid of the type alias for typedesc.

@Araq Araq added Low Priority and removed Language design labels Apr 20, 2019

@Araq

This comment has been minimized.

Copy link
Member

commented Apr 20, 2019

Low prio now that the manual has been updated.

@mratsim

This comment has been minimized.

Copy link
Collaborator

commented Apr 21, 2019

The type parameters were introduced in #7681 cc @zah

@andreaferretti

This comment has been minimized.

Copy link
Collaborator

commented Apr 23, 2019

eventually we should get rid of the type alias for typedesc

Why? It was like this before #7681, I thought that everyone was converging on removing typedesc altogether. I am fine with both choices, but going back and forth is just confusing

@krux02

This comment has been minimized.

Copy link
Contributor

commented Apr 23, 2019

Well, this is the reduced version of the problem:

template foo1(S: type; U: type): untyped =
  var myS: S
  var myU: U
  echo myS
  echo myU

foo1(int, string) # type mismatch

type is actually a hidden generic, it is type[T], the same as typedesc[T]. The secod usage of the keyword type secrectly binds to the exact same type[T]. Therefore the signature is then: template foo1[T](S: type[T]; U: type[T]): untyped. So T and U must be typesdescriptors that come from the exact same type. With typedesc this does compile, because a hack has been introduced at some point that fixes it for typedesc, but not for type.

# a hack for implicit generic types makes the magically fail
template foo1(S: type; U: type): untyped =
  var myS: S
  var myU: U
  echo myS
  echo myU

# foo1(int, string) # type mismatch

# another hack for typedesc makes this magically work
template foo2(S: typedesc; U: typedesc): untyped =
  var myS: S
  var myU: U
  echo myS
  echo myU

foo2(int, string)

# explicitly setting the generic parameters will make it work
template foo3[S,U](unused1: type[S]; unused2: type[U]): untyped =
  var myS: S
  var myU: U
  echo myS
  echo myU

foo3(int, string)


# explicitly setting the generic parameters will make it work
template foo4[S,U](unused1: typedesc[S]; unused2: typedesc[U]): untyped =
  var myS: S
  var myU: U
  echo myS
  echo myU

foo4(int, string)

# avoiding ``type`` and ``typedesc`` altogether will work as well
template foo5(S, U: untyped): untyped =
  var myS: S
  var myU: U
  echo myS
  echo myU

foo5(int, string)

template foo6(S, U: typed): untyped =
  var myS: S
  var myU: U
  echo myS
  echo myU

foo6(int, string) 

My personal recommendation is to use untyped or typed to pass types to templates and macros and avoid this weird logic in the compiler altogether.

@Araq

This comment has been minimized.

Copy link
Member

commented Apr 23, 2019

Why? It was like this before #7681, I thought that everyone was converging on removing typedesc altogether. I am fine with both choices, but going back and forth is just confusing

Sorry, I can understand you. However, I never liked it, I accepted the community vote but now that we've seen that it doesn't even work I am using my BDFL veto right, typedesc is here to stay, type is for type sections and in the meantime we also got typeof for further clarity.

@zah

This comment has been minimized.

Copy link
Member

commented Apr 25, 2019

The only problem here is that type acts as a bindOnce type at the moment. This is quite easy to fix. In the meantime, it's also possible to work-around the issue by introducing a distinct prefix in the template signature:

template foo1(S, U: distinct type): untyped =
  var myS: S
  var myU: U
  echo myS
  echo myU

Why should we spend time reverting work and introducing more rules in the compiler, only to make the language slightly less elegant? There are no real problems with the lack of clarity between type, typedesc and typeof and actually having a simple typeof operator is slightly less expressive than the current approach that allows you to add additional checks when you obtain the type of an expression:

type Key = type[Hashable](foo.bar)
@Clyybber

This comment has been minimized.

Copy link
Contributor

commented Apr 25, 2019

@zah I agree, IMO type should be the one to stay.

@Araq

This comment has been minimized.

Copy link
Member

commented Apr 29, 2019

Why should we spend time reverting work and introducing more rules in the compiler, only to make the language slightly less elegant?

Because it's more elegant to have different names for different things. We will not agree on what is more elegant, I know.

Btw, I have no idea what this means:

type Key = type[Hashable](foo.bar)
@bluenote10

This comment has been minimized.

Copy link
Contributor

commented Apr 29, 2019

@zah Initially I preferred the unification, but from a practical perspective I prefer @Araq's argument. The term typedesc is really good to grep for in a codebase, searchable on google, github issues, the forum, or irc logs. It always refers to the meaning "type as an argument". The term type is semantically ambiguous, and it requires extra effort to identify its context.

Number of github issues matching type: 718
Number of github issues matching typedesc: 78

@krux02

This comment has been minimized.

Copy link
Contributor

commented Apr 29, 2019

@zah As a guy who has to fix issues in the compiler, I really prefer the typedesc over the overload of the keyword type. For practical reasons alone.

The only problem here is that type acts as a bindOnce type at the moment. This is quite easy to fix. In the meantime, it's also possible to work-around the issue by introducing a distinct prefix in the template signature:

Please don't introduce that abuse of the distinct keyword. As the guy who is fixing the bugs in the compiler I can only see a bunch of problems when that one get's introduced.

Why should we spend time reverting work and introducing more rules in the compiler, ...

We should not have spent time on this "unification" in the first place. Simply because the distinction between type and typedesc never came up as a problem. The opposite is the case (see @bluenote10's comment)

... only to make the language slightly less elegant?

Elegance is subjective. I don't see the unification of type and typedesc more elegant at all.

@zah

This comment has been minimized.

Copy link
Member

commented Apr 29, 2019

Please don't introduce that abuse of the distinct keyword. As the guy who is fixing the bugs in the compiler I can only see a bunch of problems when that one get's introduced.

To clarify a possible misunderstanding, the use of distinct as a discriminator between bind-once and bind-many types has been present since the introduction of implicit generic params. Calling it "abuse" is a bit overly negative.

Otherwise, you are entitled to your opinion and I respect that, but I hope we won't see too disruptive reverts of working functionality. At Status, we have accumulated quite a lot of code using the type modifiers.

@krux02

This comment has been minimized.

Copy link
Contributor

commented Apr 29, 2019

@zah

... I hope we won't see too disruptive reverts of working functionality.

I hope for the same. I don't like disruptive as well.

At Status, we have accumulated quite a lot of code using the type modifiers.

I am really sorry for that. But I can give you a clear example of why we should distinguish between typedesc and type very clearly. I just fixed a bug right now. The current compiler for example just crashes in the following example:

let something = (a: 1, b: int)

I can introduce an error message where I say that a member of type typedesc[int] is illegal as a tuple member. I can also simply say typedesc is illegal. But can clearly not say that type int is illegal, because that would be plain wrong, an int is legal, because 1 is legal. And an error message that says "type is illegal" would also be very confusing.

How would you formulate this error message when you are not allowed to say typedesc?

EDIT:
@zah I accidentally edited your comment instead of replying. Sorry for that, that was an accident. I reverted it back.

@zah

This comment has been minimized.

Copy link
Member

commented Apr 30, 2019

Whatever sentence you formulate, it's trivial to replace typedesc with type symbol or type value. For example, the error message could be:

Using type values is illegal in a run-time context (i.e. as a tuple field)

If you look at the manual and tutorial modifications introduced with the typedesc reforms, I would argue that both resulted in a more clear and natural language.

@zah

This comment has been minimized.

Copy link
Member

commented Apr 30, 2019

@Araq, to quote the manual:

type operator

You can obtain the type of a given expression by constructing a type
value from it (in many other languages this is known as the typeof:idx:
operator):

  var x = 0
  var y: type(x) # y has type int

You may add a constraint to the resulting type to trigger a compile-time error
if the expression doesn't have the expected type:

  var x = 0
  var y: type[object](x) # Error: type mismatch: got <int> but expected 'object'

Clyybber added a commit to Clyybber/Nim that referenced this issue May 3, 2019

Clyybber added a commit to Clyybber/Nim that referenced this issue May 4, 2019

@narimiran narimiran closed this in 4fd79f5 May 5, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.