nim-lang / Nim

Nim is a statically typed compiled systems programming language. It combines successful concepts from mature languages like Python, Ada and Modula. Its design focuses on efficiency, expressiveness, and elegance (in that order of priority).
https://nim-lang.org
Other
16.55k stars 1.47k forks source link

Generic $ should work on everything #8149

Closed timotheecour closed 5 years ago

timotheecour commented 6 years ago

in D, toString always works no matter what variable we pass in; it'll reuse a user defined toString on the appropriate (sub-)fields if it has been defined (eg via string toString() {...} or via the new more efficient way https://dlang.org/changelog/2.079.0.html#toString)

In nim this isn't the case:

import os
import nre

proc replaceMatch(a:RegexMatch):auto=
  let temp = a.captures
  # ok
  echo a.captures[0]
  # ERROR
  echo a.captures
  return a.captures[0]

proc test()=
  let a = replace("foo", re"(\w+)", replaceMatch)

causes:

Error: type mismatch: got <Captures>
but expected one of:
...

Is there any technical reason for this limitation? the generic stringification operator$(...) could in theory work with anything, reusing user defined $(...) where appropriate, otherwise using rules:

NOTE: for types, not sure what should be default behavior when typetraits is not imported; maybe error is OK as currently done; that's an edge case arguably.

ghost commented 6 years ago

There's repr which will work for any type

timotheecour commented 6 years ago

while repr is useful for certain use cases, it's not fullfilling the use case I'm referring to (fullfilled by D's toString), as it doesn't use user defined $() on (sub-)fields when available:

in the above example echo a.captures.repr gives:

[pattern = ref 0x10db0d080 --> [pattern = 0x10db100a8"(\\w+)",
pcreObj = ref 0x7fb860f003a0 --> [],
pcreExtra = ref 0x7fb860f00400 --> [flags = 1,
study_data = 0x7fb860f00440,
match_limit = 0,
callout_data = nil,
tables = nil,
match_limit_recursion = 0,
mark = nil,
executable_jit = nil],
captureNameToId = [data = 0x10db0f668[[Field0 = 0,
Field1 = nil,
Field2 = 0], [Field0 = 0,
Field1 = nil,
Field2 = 0], [Field0 = 0,
Field1 = nil,
Field2 = 0], [Field0 = 0,
Field1 = nil,
Field2 = 0], [Field0 = 0,
Field1 = nil,
...
Araq commented 6 years ago

generic stringification $() should work with everything

It really shouldn't, even the existing implementation for object is error prone and should be removed.

timotheecour commented 6 years ago

It really shouldn't, even the existing implementation for object is error prone and should be removed.

could you provide an example for illustration?

How about the following instead (with no change of behavior, would require explicit calling of toString): instead of $(expr), define toString(expr) (or whatever suitable name) with pseudo-code:

proc toString(expr, options = optionDefault):string=
  if $(expr) compiles: return $(expr)
  if expr is an object: return "(" & fields(expr).mapIt(toString(it, options).join(", ") & ")"
  # etc: handle other cases (seq, tuples, etc)

it's an extremely useful feature in D for development / debugging (esp in case of using third party libraries that don't have custom $() defined on certain types)

later, toString could be extended (as shown above) with an options argument that would customize:

I actually had implemented a subset of these in D, was quite useful for debugging

GULPF commented 6 years ago

Related: #8023

bluenote10 commented 6 years ago

The underlying problem here is that $ fails for distinct objects:

type
  A = object
    x, y: float
  B = distinct A

let a = A(x: 0, y: 0)
let b = B(a)

echo a # works
echo b # fails

it's an extremely useful feature in D for development / debugging (esp in case of using third party libraries that don't have custom $() defined on certain types)

+1 comparing languages that do vs do not have this feature, it makes debugging so much more efficient. I still hope the issues with $ can be solved rather than going back to the C++ like experience where you typically can't debug print anything.

dom96 commented 6 years ago

Indeed, $ should work for everything. We've discussed this in #8023 (which proposed removing $ for objects). I will close #8023 in favour of this issue.

Araq commented 6 years ago

I've hit the bug twice now that my own $ failed to be imported and the compiler used system.$ instead which is wrong.

Araq commented 6 years ago

IMO the decision has been made: $ for objects should remain and we should add $ for even more generic things (ref types for example).

Ha, no. :-) But sure, lets continue the discussion here.

andreaferretti commented 6 years ago

I've hit the bug twice now that my own $ failed to be imported and the compiler used system.$ instead which is wrong.

That should be pretty evident: the first thing one does when print looks weird is to look at $, and then it should be easy to notice that it is not exported.

Araq commented 6 years ago

That should be pretty evident: the first thing one does when print looks weird is to look at $, and then it should be easy to notice that it is not exported.

It was actually rather hard to figure out, but I agree, usually it's obvious. But I'd rather detect it at compile-time anyway. Plus I don't need $ for objects for debugging, I can use repr instead.

zah commented 6 years ago

One problem that I've been running into lately is that with the introducing of func, echo is now much more often disallowed due to its side-effects. The solution is to use debugEcho, which is a bit long to type. I think the best solution is to introduce a short alias for debugEcho (it may be called dbg), which uses repr under the hood.

kayabaNerve commented 6 years ago

I only found out about this continuation of issue #8023 after I posted there. I want to repost my comment in the proper location.


I'd like to comment on this as I am having frustrations with this myself and I was late to the initial conversation. It sounds like people were against moving it to debug() and I can't follow the final statement about repr().

What if we didn't disable it but didn't force it on users? We could put it under a pragma. {.supplyDefault$.}

Having any user defined object supplied a $ operator is both a neat debugging tool but also an annoyance, especially if it sometimes overrides user supplies $s (unless that was fixed). That said, this also conflicts with Nim's design of having a strong static typing system and isn't intuitive. It's broken my code a few times now...

Can we AT LEAST have a command line/nimscript option to disable these? It's extremely annoying to have to declare proc $(x: myType) {.error.} and if the default does override the user defined one in certain cases, it isn't even guaranteed to work.

Araq commented 6 years ago

In my own codebases I'm considering to use display and don't overload $ but it's nicer if I don't have to think about display vs $, so the default $ for object remains wrong IMO.

krux02 commented 5 years ago

Araq commented on 1 Jul 2018

I've hit the bug twice now that my own $ failed to be imported and the compiler used system.$ instead which is wrong.

Araq commented on 2 Jul 2018

It was actually rather hard to figure out, but I agree, usually it's obvious. But I'd rather detect it at compile-time anyway. Plus I don't need $ for objects for debugging, I can use repr instead.

I am linking this issue here because I think that is exactly what happened. I think we really have to investigate and find a better way to do overload resolution. I think it's possible. At least for $ we can introduce warnings in the compiler that should help to ensure the correct ovorload of $ is always visible, but that is not a general solution.

Regarding repr: In the past you claimed that repr should be deprecated, as it has it's own set of problems. How can repr be deprecated when it is still essential? Well I guess you changed your opinion about this aspect. I still argue that a generic overload of $ is a much better solution we just need a better mechanism for overload resouliton so that custom overloads of $ won't be ignored by the compiler anymore.

dom96 commented 5 years ago

IMO this should be closed. We don't want $ to work for everything precisely because of this overloading issue, but also because I don't want types that I define to have an implicit $, we need another function for this or to just be happy with repr.

andreaferretti commented 5 years ago

@dom96

Indeed, $ should work for everything

We don't want $ to work for everything

Uh?

dom96 commented 5 years ago

Good catch. But people are allowed to change their mind :)

krux02 commented 5 years ago

Actually I do want $ to work on everything. I think the overloading issues is a real problem, but I think it can be solved. It's just not easy though. But I don't need this issue though to remind me.