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

Nim beginner's feedback: "echo type(1)" does not work #5827

Closed
markus-oberhumer opened this issue May 17, 2017 · 9 comments

Comments

Projects
None yet
6 participants
@markus-oberhumer
Copy link
Contributor

commented May 17, 2017

I've recently introduced Nim to a number of colleagues (mostly with a C++ and Go background), and I want to share some of the feedback I got.

Surprise: you cannot print the type of an expression

echo 1
echo type(1)    # does not compile

This seems easy to fix with the following patch (not sure if that is correct), but I wonder why something like this not enabled by default given that the type() proc is exported from system.nim.

diff --git a/lib/system.nim b/lib/system.nim
index 9b41253..ce41373 100644
--- a/lib/system.nim
+++ b/lib/system.nim
@@ -168,6 +168,8 @@ proc `type`*(x: untyped): typeDesc {.magic: "TypeOf", noSideEffect, compileTime.
   ## Builtin 'type' operator for accessing the type of an expression.
   ## Cannot be overloaded.
   discard
+from typetraits import nil
+proc `$`*(t: typedesc): string = typetraits.name(t)
 
 proc `not` *(x: bool): bool {.magic: "Not", noSideEffect.}
   ## Boolean not; returns true iff ``x == false``.

cdunn2001 added a commit to cdunn2001/Nim that referenced this issue May 18, 2017

cdunn2001 added a commit to cdunn2001/Nim that referenced this issue May 18, 2017

cdunn2001 added a commit to cdunn2001/Nim that referenced this issue May 18, 2017

cdunn2001 added a commit to cdunn2001/Nim that referenced this issue May 18, 2017

@cdunn2001

This comment has been minimized.

Copy link
Contributor

commented May 18, 2017

@markus-oberhumer, I know that araq does not like to import modules into system. Is it reasonable to expect import typetraits before $ can work on types? I think so.

See #5839

@krux02

This comment has been minimized.

Copy link
Contributor

commented May 18, 2017

I looked at the typetraits module. Everything from typetraits is implemented with magics and it only provides four procedures that are defined on typedescriptors. I suggest moving all methods from typetraits to system. typetraits can still hang around as an empty module that reexports the system procedures it had earlier for backward compatibility, but the import should be deprecated. I don't think there will be any mentionable cost to doing that.

I constantly forget the name of the typetraits module whenever I need to print the name of a typedesc.

@markus-oberhumer

This comment has been minimized.

Copy link
Contributor Author

commented Oct 30, 2017

This seems to be addressed by git commit 9565da1.

But given that proc type()*: (x: untyped) is in module system I still think that everything from typetraits should be moved to system.

@dom96

This comment has been minimized.

Copy link
Member

commented Oct 30, 2017

I personally think that the system module is too large currently and that we should be moving things out of it, not into it.

What would be better is making the compiler smart enough to give you suggestions of procedures from other modules that fit your call.

@krux02

This comment has been minimized.

Copy link
Contributor

commented Oct 30, 2017

Yes I think that this smartness of the compiler would be very great, but that would be another issue.

And you might be right that there is some unnecessary stuff in the system module that could also be in another module, but to stringify a type is not one of those bloat functions, it just improves accessability.

Things where I think they are not necessary in the system module are more of these things:

cint, cstring, ...
stmt, expr, ...  (deprecated)
PFrame, TFrame  (debuger api, should be debugger package)
on, off, ...  (just there to allow inconsistent boolen names)
internalNew
ze, ze64, ...,  (Nim has now proper unsigned integers, you never need this)
+%, *%, ... (Nim has proper unsigned integers, you never need this)

@dom96 dom96 referenced this issue Dec 23, 2017

Closed

system cleanup #6967

@dom96 dom96 added the Stdlib label Jan 17, 2018

dom96 added a commit that referenced this issue Jan 17, 2018

dom96 added a commit that referenced this issue Jun 10, 2018

@kaushalmodi

This comment has been minimized.

Copy link
Contributor

commented Jun 13, 2018

I'd love to get this merged!

@dom96 dom96 added the Hacktoberfest label Oct 2, 2018

@narimiran

This comment has been minimized.

Copy link
Member

commented Oct 4, 2018

What's the status of this one?

@krux02

This comment has been minimized.

Copy link
Contributor

commented Oct 4, 2018

I think nothing has changed here, @Araq does not want it in system.

@dom96

This comment has been minimized.

Copy link
Member

commented Oct 5, 2018

Relevant PR #7100

dom96 added a commit that referenced this issue Oct 22, 2018

@krux02 krux02 removed the Hacktoberfest label Nov 12, 2018

timotheecour added a commit to timotheecour/Nim that referenced this issue Dec 21, 2018

timotheecour added a commit to timotheecour/Nim that referenced this issue Dec 21, 2018

timotheecour added a commit to timotheecour/Nim that referenced this issue Dec 23, 2018

timotheecour added a commit to timotheecour/Nim that referenced this issue Dec 28, 2018

@Araq Araq closed this in #10071 Dec 30, 2018

Araq added a commit that referenced this issue Dec 30, 2018

revives: Move typetraits.`$` to system. Fixes #5827 (#10071)
* Move typetraits.`$` to system. Fixes #5827.
* revive PR; adjust code to make sure everything works and add tests
* fix tests/concepts/tstackconcept.nim
* address comments
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.