robpike / ivy

ivy, an APL-like calculator
Other
1.32k stars 103 forks source link

scanner doesn't understand go 1.13 number syntax #110

Closed wgrr closed 2 years ago

wgrr commented 2 years ago

i quickly gonne through the code to understand this so correct me if i'm wrong, i think who do the job in ivy to actually parse the number is the parser with Go's strconv.ParseInt, but that changed in go 1.13, so such input to ivy does nothing, no errors neither yields results.

1_0 ** 2
wgrr commented 2 years ago

that seens to be the case, with this quick change the above input yields correctly 100

diff --git a/scan/scan.go b/scan/scan.go
index e63bded..6bd6d6d 100644
--- a/scan/scan.go
+++ b/scan/scan.go
@@ -492,7 +492,7 @@ func (l *Scanner) scanNumber(followingSlashOK, followingJOK bool) bool {
        // We can't set it to 8 in case it's a leading-0 float like 0.69 or 09e4.
    }
    l.acceptRun(digits)
-   if l.accept(".") {
+   if l.accept(".") || l.accept("_") {
        l.acceptRun(digits)
    }
    if l.accept("eE") {

my question is, @robpike you think ivy should use the number syntax of go1.13 or keep the current and replace strconv.ParseInt ?

robpike commented 2 years ago

I might be talked into it but I don't think it's very important to add this ability. It's actually rather fiddly to do this right - your change isn't enough. The right fix would be to add the underscore to the constants used in digitsForBase. It's probably OK not to worry about all the corner cases but let the strconv routines do the heavy lifting, which is what's done now anyway.

Still, does this matter?

What bothers me more is the lack of an error in your example. That's a problem.

robpike commented 2 years ago

I found and fixed the scanner problem in commit be03b1486a4aed05119fc8f4f1e6a7625082e311

wgrr commented 2 years ago

the issue for me was the lack of an error too, i thought strconv.ParseInt was the problem because i had a working build from go 1.11 (yeilding an error) and go1.17 doesn't, apparently i was wrong. be03b14 fixes the problem.