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.57k stars 1.47k forks source link

Funny issue with default proc parameter #15895

Closed StefanSalewski closed 3 years ago

StefanSalewski commented 3 years ago
proc newCheckButton*() =
  echo "newCheckButton"

proc newCheckButton*(label: cstring = "") =
  echo "newCheckButton"

proc ok*() =
  echo "ok"

ok()
#newCheckButton()

Compiles and runs fine. But when we uncomment newCheckButton() call:

$ nim c --gc:arc t.nim 
Hint: used config file '/home/salewski/Nim/config/nim.cfg' [Conf]
Hint: used config file '/home/salewski/Nim/config/config.nims' [Conf]
....
/tmp/hhh/t.nim(11, 15) Error: ambiguous call; both t.newCheckButton() [declared in /tmp/hhh/t.nim(1, 6)] and t.newCheckButton(label: cstring) [declared in /tmp/hhh/t.nim(4, 6)] match for: ()

Of course it is not a real bug, but it is ugly as it hides until someone tests.

Araq commented 3 years ago

So test your code, I don't have a good solution either.

StefanSalewski commented 3 years ago

Testing 10k procs is hard. And prebuilt packages would not help in this case.

Unfortunately this behaviour destroys some of the advantages that a compiled languages offer, that is that the compiler can find obvious errors in advance. So we have to really use each function to ensure that it really compiles.

We just got a new issue as a result of allowing optional var out parameters as suggested by demotomohiro:

https://github.com/StefanSalewski/gintro/issues/102#issuecomment-726454448

proc getCoords*(self: Event; xWin: var cdouble = cast[ptr cdouble](nil)[];
    yWin: var cdouble = cast[ptr cdouble](nil)[]): bool =
  toBool(gdk_event_get_coords(cast[ptr Event00](self.impl), xWin, yWin))

proc getCoords*(self: Event): (cdouble, cdouble) =
  if not toBool(gdk_event_get_coords(cast[ptr Event00](self.impl), result[0], result[1])):
    raise newException(Defect, "This event don't has a coordinate field.")
tmp/examples/gtk3/simpledrawingarea.nim(157, 21) Error: ambiguous call; both gdk.getCoords(self: Event) [proc declared in /home/salewski/.nimble/pkgs/gintro-#head/gintro/gdk.nim(12609, 6)] and gdk.getCoords(self: Event, xWin: var cdouble, yWin: var cdouble) [proc declared in /home/salewski/.nimble/pkgs/gintro-#head/gintro/gdk.nim(6233, 6)] match for: (EventMotion)

But maybe we will remove the optional out parameters again, as it may generate more trouble than benefit.

Araq commented 3 years ago

Unfortunately this behaviour destroys some of the advantages that a compiled languages offer, that is that the compiler can find obvious errors in advance. So we have to really use each function to ensure that it really compiles.

It really doesn't destroy the advantage, if it doesn't compile, it's still not a runtime crash but that you have to track down.

Testing 10k procs is hard. And prebuilt packages would not help in this case.

It would have helped. Your way: Generate 10K procs and hope for the best. My way: c2nim the procs that one is likely to need. Review the resulting Nim code, see what the C code actually means. (The old "is T* a pointer or an array and can it be NULL" problem.) Add more wrappers when you or your users need them.

But much like you generate 10k procs you can also generate test cases.

StefanSalewski commented 3 years ago

Add more wrappers when you or your users need them.

That does not work in real life for such large libs. Users have requested libhandy, libnice, libvte and libgranite support, all libs with gobject-introspection support but I have no idea what they are. I have provided the first 3, as they looked like serious demands, but ignored libgranite for now. And for the core GTK libs -- no one would intend to use them if they would be not complete. I myself would never consider using incomplete bindings -- not for Qt, not for OpenGl or Vulkan or whatever. It is OK when a few procs are missing, but 99% should just work.

For prebuilt libs: I had a discussion with Mr Droege from the Rust GTK team. It seems to be indeed possible to distribute prebuilt bindings. But of course we need then Windows and Mac machines, 32 and 64 bit, and then we have to compare all the bindings and use a set of when statements to join all into one. But Rust has a much larger GTK team currently.