Implement generic fromBin|Oct|Hex in strutils #11107

avitkauskas · 2019-04-24T20:33:05Z

Implement all families of proc like parseBinInt8, parseBinInt16, ..., parseBinUint64 (and Oct|Hex) in strutils returning their respective types of integer.

kaushalmodi · 2019-04-25T13:06:46Z

Is it possible to design generics for this? I don't like the idea of hardcoding types (int, uint, etc) in proc names.

kaushalmodi · 2019-04-25T13:07:47Z

There was an awesome effort put in by @timotheecour a while back: #10346, but unfortunately they PR got closed.

See nim-lang/RFCs#84.

avitkauskas · 2019-04-25T13:30:23Z

@kaushalmodi I think this could be easily done by:

import parseutils

proc parseFromBin*[T: SomeInteger](v: var T, s: string) =
     let p = parseutils.parseBin(s, v)
     if p != s.len or p == 0:
       raise newException(ValueError, "invalid binary integer: " & s)

var bin: int8
bin.parseFromBin("0b11011")

doAssert bin == 0b11011

... or something. But there is already the proc parseBinInt in stdlib, so I wanted just to expand this to other types. Actually parsutils.parseBin is now generic and does almost the same, but without the Exception guard.

kaushalmodi · 2019-04-25T14:37:00Z

But there is already the proc parseBinInt in stdlib

Then may be that should be deprecated? Asking @Araq to weigh in on this.

avitkauskas · 2019-04-25T20:34:15Z

Mistery - koch changes oct literal when generating doc examples!

my code in strutils.nim:

runnableExamples:
    let a = "0o_123_456_777"
    doAssert a.parseOctInt8() == 0o377'i8

./koch doc generates in example file:

block:
  let a = "0o_123_456_777"
  doAssert a.parseOctInt8() == 0o777'i8  # <--- see this!

and fails with the error:

strutils_examples29.nim(9, 32) Error: number out of range: '0o777'i8'

If I try 0o277'i8 it fails with 0o677'i8.
Is that my problem, or koch?

mratsim · 2019-04-26T12:59:41Z

0o777 is 0x1FF in hexadecimal, a byte can represent from 0x00 to 0xFF. And 0xFF is 0o377 so the output is correct modulo the int8 range.

avitkauskas · 2019-04-26T13:21:44Z

The thing is - I put in runableExamples 0o377'i8 (and that's 0xFF, correct), but koch doc when generating examples changes this to 0o777'i8 in the generated examples file, and then of course tests fail with the error message Error: number out of range: '0o777'i8'.

But how can it be that this change happens while extracting my code into separate examples file?
All other lines arround this one are transfered to example file verbatim without any changes.

Araq · 2019-04-29T15:28:24Z

But how can it be that this change happens while extracting my code into separate examples file?

Well the "extraction" is ... er... special. :-)

data-man · 2019-04-29T15:44:10Z

parseXIntX in the docs, but parseXintX by fact.
Isn't good especially for autocompletion.

avitkauskas · 2019-04-29T18:09:15Z

@data-man Yes, you are right, there would be a problem for autocompletion as you say.
So, maybe it would be much easier and better then to introduce procs like:

proc fromBin*[T: SomeInteger](v: var T, s: string) =
     let p = parseutils.parseBin(s, v)
     if p != s.len or p == 0:
       raise newException(ValueError, "invalid binary integer: " & s)

var x: int8
x.fromBin("0b11011")

Especially when there are already procs toBin etc. in strutils.

And then we could probably deprecate parseBinInt if we get the agreement on that.

data-man · 2019-04-29T18:51:48Z

Personally I'd prefer universal parseInt with base parameter. And deprecate others.
What if someone wants to add a parseTernaryInt and family tomorrow? Or/and parseSeptenaryInt and family?
strutils is already big enough.

mratsim · 2019-04-30T11:41:20Z

They add this parseTernaryInt in their own library just like I did here: https://github.com/status-im/nim-stint/blob/6853ebe97c21426952ce64c2df6278514797dc3f/stint/io.nim#L116-L117

func parse*[bits: static[int]](input: string, T: typedesc[Stuint[bits]], radix: static[uint8] = 10): T =
  ## Parse a string and store the result in a Stuint[bits].

func parse*[bits: static[int]](input: string, T: typedesc[Stint[bits]], radix: static[int8] = 10): T =
  ## Parse a string and store the result in a Stint[bits] or Stuint[bits].

Araq · 2019-05-02T06:20:45Z

lib/pure/strutils.nim

@@ -1068,44 +1068,145 @@ proc parseFloat*(s: string): float {.noSideEffect, procvar,

 proc parseBinInt*(s: string): int {.noSideEffect, procvar,
  rtl, extern: "nsuParseBinInt".} =
-  ## Parses a binary integer value contained in `s`.
+  ## Parses a binary integer value contained in `s` and returns `int`.


"And returns int"? Do we need to assume nobody can read Nim type declarations?

Araq · 2019-05-02T06:21:24Z

I appreciate the effort but this should use generics indeed. You can then deprecate the non-generic variant or simply leave it as it is.

avitkauskas · 2019-05-05T18:17:14Z

@Araq When talking about generics, what variant would you prefer:

proc fromBin*[T: SomeInteger](x: var T, s: string) =
  let p = parseutils.parseBin(s, x)
  if p != s.len or p == 0:
    raise newException(ValueError, "invalid binary integer: " & s)

var x: int8
x.fromBin("0b11011")

or

proc fromBin*[T: SomeInteger](s: string): T =
  let p = parseutils.parseBin(s, result)
  if p != s.len or p == 0:
    raise newException(ValueError, "invalid binary integer: " & s)

var x: int8
x = fromBin[int8]("0b11011")

# by the way, x = "0b11011".fromBin[int8] would not work: 
# cannot instantiate: 'T' and other errors

Araq · 2019-05-08T09:27:43Z

This variant:

proc fromBin*[T: SomeInteger](s: string): T =
  let p = parseutils.parseBin(s, result)
  if p != s.len or p == 0:
    raise newException(ValueError, "invalid binary integer: " & s)

var x: int8
x = fromBin[int8]("0b11011")

# by the way, x = "0b11011".fromBin[:int8] would work perfectly fine.

mratsim · 2019-05-09T08:38:52Z

I feel like we're back to typedesc vs generics API design here with a sprinkle of backward compatibility concerns (nim-lang/RFCs#40)

Anyway, I would be happy to merge the non-controversial state from here: c1d7ea9 and discuss/split the later commits with API issues in a separate PR.

Araq · 2019-05-09T10:25:42Z

I feel like we're back to typedesc vs generics API design here with a sprinkle of backward compatibility concerns (nim-lang/RFCs#40)

The stdlib uses [T] and so that's what should be used. I can find an RFC for everything that could be used to delay further progress.

avitkauskas · 2019-05-12T09:37:07Z

runnableExamples tests will not pass until #11131 is fixed.

avitkauskas · 2019-05-12T11:56:58Z

What should I do about that?

Hint: test_vectors [Processing]
test_vectors.nim(142, 27) Error: ambiguous call; both utils.fromHex(a: string) [declared in ..\..\..\..\..\Users\appveyor\.nimble\pkgs\nimcrypto-0.3.9\nimcrypto\utils.nim(158, 6)] and strutils.fromHex(s: string) [declared in ..\..\..\lib\pure\strutils.nim(1059, 6)] match for: (string)

narimiran · 2019-05-14T09:09:38Z

runnableExamples tests will not pass until #11131 is fixed.

This is fixed now.

What should I do about that?

Where does test_vectors.nim come from? I didn't find it in Nim repo, is it some Nimble package?

avitkauskas · 2019-05-14T12:21:30Z

@narimiran, it's in nimcrypto-0.3.9 package as I understand.

narimiran · 2019-05-14T13:30:00Z

it's in nimcrypto-0.3.9 package as I understand.

Ah, I found it. It is blscurve package that is failing, and here is test_vectors.nim

test_vectors.nim(142, 27) Error: ambiguous call; both utils.fromHex(a: string) [declared in nimcrypto\utils.nim(158, 6)] and strutils.fromHex(s: string) [declared in lib\pure\strutils.nim(1059, 6)] match for: (string)

This can then be fixed by patching the mentioned package by having this line:

import strutils except fromHex

mratsim · 2019-05-18T12:02:57Z

Merged status-im/nim-blscurve#21 and retriggering CI. Can we put Appveyor under a Nim organisation. I can restart Travis builds but not Appveyor's.

sophisticate364 · 2019-05-18T12:06:07Z

Why is it merging my account ??and they gain access thru making URLs

…

________________________________ From: Mamy Ratsimbazafy <notifications@github.com> Sent: May 18, 2019 6:02:19 AM To: nim-lang/Nim Cc: Subscribed Subject: Re: [nim-lang/Nim] Implement generic fromBin|Oct|Hex in strutils (#11107) Closed #11107<https://eur04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit.luolix.top%2Fnim-lang%2FNim%2Fpull%2F11107&data=02%7C01%7C%7C7f28bed743804780f39608d6db88ae8b%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C636937777414769462&sdata=qY3UEshYfKHoLLGmhQGs4hDgFPkYde6BgVPJzvEDq3w%3D&reserved=0>. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub<https://eur04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit.luolix.top%2Fnim-lang%2FNim%2Fpull%2F11107%3Femail_source%3Dnotifications%26email_token%3DALVKHDX4LEUGQ3HE2ZWYEG3PV7V4XA5CNFSM4HIHVAA2YY3PNVWWK3TUL52HS4DFWZEXG43VMVCXMZLOORHG65DJMZUWGYLUNFXW5KTDN5WW2ZLOORPWSZGORQQII2I%23event-2350941289&data=02%7C01%7C%7C7f28bed743804780f39608d6db88ae8b%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C636937777414769462&sdata=WkkVvOFDfSJQ1uUv7g0b5uuM7bNbnFDFgP83%2BuGLr5s%3D&reserved=0>, or mute the thread<https://eur04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit.luolix.top%2Fnotifications%2Funsubscribe-auth%2FALVKHDQAFSQSGJ72ANEE3MLPV7V4XANCNFSM4HIHVAAQ&data=02%7C01%7C%7C7f28bed743804780f39608d6db88ae8b%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C636937777414779474&sdata=VTRO%2BOvXvEMKJYXCi32d0QnNoFbAidVo%2FlZWMPu%2FbG0%3D&reserved=0>.

avitkauskas · 2019-05-22T19:12:16Z

This is ready for review.

avitkauskas added 10 commits April 19, 2019 21:32

make parsutils.parseBin generic and improve runnableExamples

fa82766

reimplement parseBin/Oct/Hex and improve runnableExamples

0dcdbbe

update changelog.md file with parseBin/Oct/Hex and fix a typo

1e489d6

delete template and code parseBin|Oct|Hex separately

ffebf98

split definitions between int and other SomeInteger

dbcf880

make parseBin|Oct|Hex generic and remove rtl pragmas

c1d7ea9

Merge remote-tracking branch 'upstream/devel' into parse-to-num

914d20f

remove pragma inline from praseBin|Oct|Hex

6886c02

implement families of parse(Bin|Oct|Hex)Int[xx]

5de414a

fix typo [skip ci]

47f8245

mratsim mentioned this pull request Apr 28, 2019

Nim doc mangles signed octal literals #11131

Closed

Araq reviewed May 2, 2019

View reviewed changes

implement generic fromBin|Oct|Hex

9a0b591

avitkauskas changed the title ~~Implement families of parse(Bin|Oct|Hex)Int[xx] in strutils~~ Implement generic fromBin|Oct|Hex in strutils May 12, 2019

make runnableExamples shorter

d55bd6f

avitkauskas mentioned this pull request May 17, 2019

Fix nimcrypto/utils.fromHex conflict with the new strutils.fromHex status-im/nim-blscurve#21

Merged

mratsim closed this May 18, 2019

mratsim reopened this May 18, 2019

Araq merged commit 981f957 into nim-lang:devel May 23, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement generic fromBin|Oct|Hex in strutils #11107

Implement generic fromBin|Oct|Hex in strutils #11107

avitkauskas commented Apr 24, 2019 •

edited

Loading

kaushalmodi commented Apr 25, 2019

kaushalmodi commented Apr 25, 2019 •

edited

Loading

avitkauskas commented Apr 25, 2019 •

edited

Loading

kaushalmodi commented Apr 25, 2019

avitkauskas commented Apr 25, 2019 •

edited

Loading

mratsim commented Apr 26, 2019 •

edited

Loading

avitkauskas commented Apr 26, 2019 •

edited

Loading

Araq commented Apr 29, 2019

data-man commented Apr 29, 2019

avitkauskas commented Apr 29, 2019 •

edited

Loading

data-man commented Apr 29, 2019

mratsim commented Apr 30, 2019 •

edited

Loading

Araq May 2, 2019

Araq commented May 2, 2019

avitkauskas commented May 5, 2019 •

edited

Loading

Araq commented May 8, 2019

mratsim commented May 9, 2019 •

edited

Loading

Araq commented May 9, 2019

avitkauskas commented May 12, 2019

avitkauskas commented May 12, 2019 •

edited

Loading

narimiran commented May 14, 2019

avitkauskas commented May 14, 2019

narimiran commented May 14, 2019

mratsim commented May 18, 2019

sophisticate364 commented May 18, 2019 via email

avitkauskas commented May 22, 2019

Implement generic fromBin|Oct|Hex in strutils #11107

Implement generic fromBin|Oct|Hex in strutils #11107

Conversation

avitkauskas commented Apr 24, 2019 • edited Loading

kaushalmodi commented Apr 25, 2019

kaushalmodi commented Apr 25, 2019 • edited Loading

avitkauskas commented Apr 25, 2019 • edited Loading

kaushalmodi commented Apr 25, 2019

avitkauskas commented Apr 25, 2019 • edited Loading

mratsim commented Apr 26, 2019 • edited Loading

avitkauskas commented Apr 26, 2019 • edited Loading

Araq commented Apr 29, 2019

data-man commented Apr 29, 2019

avitkauskas commented Apr 29, 2019 • edited Loading

data-man commented Apr 29, 2019

mratsim commented Apr 30, 2019 • edited Loading

Araq May 2, 2019

Choose a reason for hiding this comment

Araq commented May 2, 2019

avitkauskas commented May 5, 2019 • edited Loading

Araq commented May 8, 2019

mratsim commented May 9, 2019 • edited Loading

Araq commented May 9, 2019

avitkauskas commented May 12, 2019

avitkauskas commented May 12, 2019 • edited Loading

narimiran commented May 14, 2019

avitkauskas commented May 14, 2019

narimiran commented May 14, 2019

mratsim commented May 18, 2019

sophisticate364 commented May 18, 2019 via email

avitkauskas commented May 22, 2019

avitkauskas commented Apr 24, 2019 •

edited

Loading

kaushalmodi commented Apr 25, 2019 •

edited

Loading

avitkauskas commented Apr 25, 2019 •

edited

Loading

avitkauskas commented Apr 25, 2019 •

edited

Loading

mratsim commented Apr 26, 2019 •

edited

Loading

avitkauskas commented Apr 26, 2019 •

edited

Loading

avitkauskas commented Apr 29, 2019 •

edited

Loading

mratsim commented Apr 30, 2019 •

edited

Loading

avitkauskas commented May 5, 2019 •

edited

Loading

mratsim commented May 9, 2019 •

edited

Loading

avitkauskas commented May 12, 2019 •

edited

Loading