nim-lang / bigints

BigInts for Nim
MIT License
124 stars 32 forks source link

Update elliptic.nim #103

Open planetis-m opened 2 years ago

planetis-m commented 2 years ago

The most significant change is turning let into const - a huge amount of temporaries goes away. Other is porting from strutils to strformat. And using 'bi when possible

planetis-m commented 2 years ago

Error: undeclared identifier: 'bi'

narimiran commented 2 years ago

Error: undeclared identifier: 'bi'

That's why we need https://github.com/nim-lang/bigints/blob/master/src/bigints/private/literals.nim, for example. See comment there: "It is needed as a workaround for Nim's parser for versions <= 1.4."

Long story short: please revert those 'bi changes.

dlesnoff commented 2 years ago

So we have to drop support for versions <= 1.4 to define const litterals with Bigints ? I appreciate the change from strutils to strformat, the echo looks nicer that way.

konsumlamm commented 2 years ago

So we have to drop support for versions <= 1.4 to define const litterals with Bigints ?

We have to drop support for Nim <= 1.4 when we want to use 'bi.

dlesnoff commented 2 years ago

I am not a fan of all those constants defined in each script. We should provide a file in the module with different constants, like zero, one, eventually the first digits up to ten, minus one, … I don’t know if these declarations takes memory, but if it does, we should make these imports optional.

planetis-m commented 2 years ago

Can you guys update the CI to 1.6 instead? Here and in different projects like the main repo, threading, etc my PRs are stalled because older versions of Nim don't have features needed.

dlesnoff commented 2 years ago

I don’t think we will remove 1.4.8 support. It has been released on 26th May and I believe we keep support for Nim versions that has been released less than one year before (or at least two Nim versions). Otherwise, I do not understand how this impact your personal projects : we do support 1.6. Is it really a needed feature here ? I think you can simply move the code between brackets into a let statement, before the echo.

planetis-m commented 2 years ago

No I meant various PRs I made in Nim are postponed because of build failures. Like https://github.com/nim-lang/Nim/pull/19360 https://github.com/nim-lang/threading/pull/6 This is pbl not a good place to discuss this.