New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Discrepancy in Documentation About 'f128 Type-Suffix #10213

Closed
tajmone opened this Issue Jan 6, 2019 · 10 comments

Comments

Projects
None yet
6 participants
@tajmone
Copy link
Contributor

tajmone commented Jan 6, 2019

There seems to be a discrepancy in User Manual: Numerical constants section regarding the 'f128 type suffix.

It's mentioned in the type suffixes Table, but it's missing in the "BNF-like" form definition (that precedes the table) where it's not mentioned at all. One of the two seems wrong.

I've come across this while editing a syntax highlighter definition for Nim, and noticed the 'f128 suffix mentioned in the table, but I also noticed that both Sublime Text and VCS Nim syntaxes ignore this suffix (not highlithed as part of the numeral), so I was wondering if it's actually in use. Is it?

In my highlighter test I'm using this way, just to test the syntax (in an approximate way):

var of4 = 0o007'f128

... that is, I'm assuming it's supported.

@zevv

This comment has been minimized.

Copy link
Contributor

zevv commented Jan 6, 2019

echo 9999999999999999.0f128 - 9999999999999998.0f128

does not compile for me:

t2.nim(2, 6) Error: system module needs: float128
@timotheecour

This comment has been minimized.

Copy link
Contributor

timotheecour commented Jan 6, 2019

you're missing a ' but, ya, it errors

echo 12'f128
/Users/timothee/git_clone/nim/timn/tests/nim/all/t0088.nim(5, 6) Error: system module needs: float128
  echo 12'f128
       ^
/Users/timothee/git_clone/nim/timn/tests/nim/all/t0088.nim(5, 6) template/generic instantiation from here
/Users/timothee/git_clone/nim/timn/tests/nim/all/t0088.nim(5, 6) template/generic instantiation from here
Error: internal error: wanted: tyFloat128 got: tyProxy
Traceback (most recent call last)
/Users/timothee/git_clone/nim/Nim/compiler/nim.nim(109) nim
/Users/timothee/git_clone/nim/Nim/compiler/nim.nim(73) handleCmdLine
/Users/timothee/git_clone/nim/Nim/compiler/cmdlinehelper.nim(92) loadConfigsAndRunMainCommand
/Users/timothee/git_clone/nim/Nim/compiler/main.nim(171) mainCommand
/Users/timothee/git_clone/nim/Nim/compiler/main.nim(78) commandCompileToC
/Users/timothee/git_clone/nim/Nim/compiler/modules.nim(134) compileProject
/Users/timothee/git_clone/nim/Nim/compiler/modules.nim(79) compileModule
/Users/timothee/git_clone/nim/Nim/compiler/passes.nim(194) processModule
/Users/timothee/git_clone/nim/Nim/compiler/passes.nim(86) processTopLevelStmt
/Users/timothee/git_clone/nim/Nim/compiler/sem.nim(609) myProcess
/Users/timothee/git_clone/nim/Nim/compiler/sem.nim(572) semStmtAndGenerateGenerics
/Users/timothee/git_clone/nim/Nim/compiler/semstmts.nim(2052) semStmt
/Users/timothee/git_clone/nim/Nim/compiler/semexprs.nim(919) semExprNoType
/Users/timothee/git_clone/nim/Nim/compiler/semexprs.nim(2573) semExpr
/Users/timothee/git_clone/nim/Nim/compiler/semstmts.nim(1992) semStmtList
/Users/timothee/git_clone/nim/Nim/compiler/semexprs.nim(2481) semExpr
/Users/timothee/git_clone/nim/Nim/compiler/semexprs.nim(2090) semMagic
/Users/timothee/git_clone/nim/Nim/compiler/semexprs.nim(900) semDirectOp
/Users/timothee/git_clone/nim/Nim/compiler/semexprs.nim(750) semOverloadedCallAnalyseEffects
/Users/timothee/git_clone/nim/Nim/compiler/semcall.nim(504) semOverloadedCall
/Users/timothee/git_clone/nim/Nim/compiler/semcall.nim(312) resolveOverloads
/Users/timothee/git_clone/nim/Nim/compiler/semcall.nim(93) pickBestCandidate
/Users/timothee/git_clone/nim/Nim/compiler/sigmatch.nim(2438) matches
/Users/timothee/git_clone/nim/Nim/compiler/sigmatch.nim(2378) matchesAux
/Users/timothee/git_clone/nim/Nim/compiler/sigmatch.nim(2189) prepareOperand
/Users/timothee/git_clone/nim/Nim/compiler/semexprs.nim(41) semOperand
/Users/timothee/git_clone/nim/Nim/compiler/semexprs.nim(2426) semExpr
/Users/timothee/git_clone/nim/Nim/compiler/magicsys.nim(82) getSysType
/Users/timothee/git_clone/nim/Nim/compiler/msgs.nim(538) internalError
/Users/timothee/git_clone/nim/Nim/compiler/msgs.nim(414) rawMessage
/Users/timothee/git_clone/nim/Nim/compiler/msgs.nim(411) rawMessage
/Users/timothee/git_clone/nim/Nim/compiler/msgs.nim(327) handleError
/Users/timothee/git_clone/nim/Nim/compiler/msgs.nim(317) quit
@tajmone

This comment has been minimized.

Copy link
Contributor Author

tajmone commented Jan 6, 2019

As far as my syntax highlighter goes, should I keep the 'f128 suffix or should I drop it?

(i.e. is it supposed to be part of Nim?)

@timotheecour

This comment has been minimized.

Copy link
Contributor

timotheecour commented Jan 7, 2019

IMO it should be part of nim, it's just a bug

echo 1.0'f64 # works
echo 1.0'f128 # bug: fails but should work
echo 1.0'f127 # ok: fails and should fail, not part of nim
@krux02

This comment has been minimized.

Copy link
Contributor

krux02 commented Jan 7, 2019

I think float128 should be removed entirely. It isn't supported properly anyway. It may stay a reserved literal type, but any information that could make people believe it is a supported type should be removed.

@timotheecour

This comment has been minimized.

Copy link
Contributor

timotheecour commented Jan 7, 2019

slightly off-topic but maybe instead of float128, what's missing is largestFloatas defined by largest FP size implemented in hardware; we have BiggestFloat but that's float64

eg, in D that'd be real and is 80 bits on x86: https://dlang.org/spec/type.html

real real.nan largest FP size implemented in hardwareImplementation Note: 80 bits for x86 CPUs or double size, whichever is larger

this allows more accurate precision in FP expressions, the temporaries being evaluated on largest available FP size instead of 64 bits, at no extra cost (IIUC)

[EDIT] actually I just found clongdouble in system.nim, but it's mapped to BiggestFloat which seems wrong since BiggestFloat is float64

lib/system.nim:1780:3:  clongdouble* {.importc: "long double", nodecl.} = BiggestFloat

see https://en.wikipedia.org/wiki/Long_double:

On the x86 architecture, most C compilers implement long double as the 80-bit extended precision type supported by x86 hardware

so we should probably remove float128, and properly implement clongdouble instead, maybe with a better name; or, if that doesn't break the world, remap BiggestFloat to clongdouble and treat it as long double (ie 80 bits on most platforms, etc)

@Araq

This comment has been minimized.

Copy link
Member

Araq commented Jan 8, 2019

D's 80 bit floating point type is widely regarded as a mistake.

@timotheecour

This comment has been minimized.

Copy link
Contributor

timotheecour commented Jan 8, 2019

D's 80 bit floating point type is widely regarded as a mistake.

I haven't heard that before; do you have a source or article explaining why it's widely regarded as a mistake?
It's nothing D specific btw, it's a C type that D exposes, and, to state the obvious, is useful for applications requiring higher accuracy FP

@narimiran

This comment has been minimized.

Copy link
Member

narimiran commented Jan 8, 2019

Btw, regardless of that float80 discussion, back to the original question: should float128 be removed from the manual?

@Araq

This comment has been minimized.

Copy link
Member

Araq commented Jan 8, 2019

Yes.

@narimiran narimiran closed this in 2589528 Jan 9, 2019

narimiran added a commit that referenced this issue Jan 9, 2019

tajmone added a commit to tajmone/highlight that referenced this issue Jan 10, 2019

Nim.lang: Drop 'f128
Nim langDef v1.1.1: Drop the `'f128` type suffix.
It turns out that, although still documented, it was no longer supported.
See discussion and commit:

nim-lang/Nim@2589528
nim-lang/Nim#10213

@narimiran narimiran added Crash and removed Crash labels Feb 12, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment