Skip to content
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

misc problems with number literals #17504

Closed
3 tasks done
timotheecour opened this issue Mar 25, 2021 · 9 comments
Closed
3 tasks done

misc problems with number literals #17504

timotheecour opened this issue Mar 25, 2021 · 9 comments

Comments

@timotheecour
Copy link
Member

timotheecour commented Mar 25, 2021

list of remaining unfixed issues from the 11 ones mentioned in #17020 (comment)

problem 1

  • all the existing nim syntax highlighters are broken and need to handle this, eg this renders badly in sublime text, github (which depends on github linguist, which itself IIRC depends on sublime text nimlime nim package) [EDIT: now works for sublime text in my fork or pending PR merging, and now works in github which uses my fork)]

problem 2

nim doc renders badly

when true:
  runnableExamples:
    func `'wrap`(a: string): string = "{" & a & "}"
    let a = -128'i8
    echo -12'wrap

the syntax highlight of `'wrap` is bad:
see
image

Note: the relevant code to fix is in lib/packages/docutils/highlite.nim eg:

    of '\'':
      inc(pos)
      g.kind = gtCharLit

problem 3

unless we backport this and upgrade csources, we won't be able to benefit from this in stdlib modules used by bootstrap (dependencies of koch, compiler, etc), and using it in other stdlib modules would make --useversion:1.2|... unusuable because when false, runnableExamples, {.since: (1,5,1).} etc cannot protect from lexer/parser errors.

However this can be largely mitigated by nim-lang/RFCs#348 (which itself needs to be backported including to csources/csources2)

problem 4

we can't use a literal proc in quote do:
this is possibly a consequence of limitation of method call syntax (can't use MCS inside templates)

when true:
  import std/macros
  macro bar(): untyped =
    func wrap1(a: string): string = "{" & a & "}"
    func `'wrap2`(a: string): string = "{" & a & "}"
    result = quote do:
      let a1 = wrap1"-128" # ok
      let a2 = -128'wrap2 # Error: undeclared field: ''wrap2' # bug

    result = genAst: # from https://github.com/nim-lang/Nim/pull/17426
      let a3 = -128'wrap2 # xxx still doesn't work
      let a4 = `'wrap2`("-128") # ok, but it's a workaround
  bar()

problem 5

problem 6

we should turn the builtin 123'f64 + friends into non-builtin reusing this feature so that compiler logic simplifies, this could be possible as follows:

when defined(nimHasCustomLiterals):
  include system/literals_builtin # defines: proc `'f64`*(a: string): float64 {.magic.}, etc
else: discard # for bootstrap; nothing to do; it's hard coded in language in this case

problem 7

logical step after bug 6: deprecate literals without apostrophe, eg:
deprecated 12f, 12i32 etc
(nim-lang/RFCs#216 also recommended deprecating those)

problem 8

  • other editors (eg vscode) need to support number literals

Additional Information

1.5.1 355985a

@timotheecour
Copy link
Member Author

timotheecour commented Mar 25, 2021

@JohnAD thanks for your PR #17020, even though it was taken over and further improved, the feature would've probably taken much longer to implement without your work!

I've migrated remaining issues to here for clarity

for bug 1:

you mentioned earlier:

Yes. I mod'd my personal version of the sublime highlighter code for my own purposes, but I'll consider doing a PR for the public NimLime also.

that would be super helpful (for bug 1 above); otherwise it's painful to read/write code with this new feature as syntax highlighting is messed up. Sublime NimLime would be great start

note that github syntax highlighting is based on it, so if NimLime is fixed, fixing github could be a simple PR away.

links

vendor/grammars/NimLime:
- source.nim
- source.nim.comment
- source.nim_filter
- source.nimcfg

@Araq
Copy link
Member

Araq commented Mar 25, 2021

You misuse the term "bug". You mean problems/issues/possible improvements/difference in taste.

@JohnAD
Copy link
Contributor

JohnAD commented Mar 26, 2021

Started work on Sublime NimLime filter: nim-lang/NimLime#153

@timotheecour timotheecour changed the title misc issues with number literals misc problems with number literals Mar 26, 2021
@timotheecour
Copy link
Member Author

You misuse the term "bug". You mean problems/issues/possible improvements/difference in taste.

changed to s/bug/problem/

of these, the only potential "difference in state" would be problem 7, but it falls better under the category "avoid 2 ways to 2 the same thing", especially when 1 way is preferred as it generalizes to user defiend literals. The question there is how much code relies on old behavior without the apostrophe, and it can be estimated via important_packages with a PR.

@JohnAD
Copy link
Contributor

JohnAD commented Mar 26, 2021

I imagine problem 7 is a breaking change on many legacy libraries in nimble/etc.

Just curious: is there a plan for Nim 2.0 in the future; or is that too far away? If one is coming out in the next year or so, this could be queued up with other desired-but-breaking changes.

(And nimble.directory could be modified so that 1.x and 2.x lookups are using different datasets.)

@timotheecour
Copy link
Member Author

breaking change

a deprecation is not a breaking change, it helps with code migration with the usual --warningAsError:X etc tools. Eventually we'll have a revived nim fix (aka nimlint, similar to go fix) to automate such changes via PR's against affected packages. If you do nothing (ie don't issue a deprecation) in 1.x and make it an error in 2.x, the migration from 1.x to 2.x will be more painful.

In any case, there's no need IMO to wait for a hypothetical nim 2.x (let alone next year) to do such changes, nim has when defined(nimLegacyX) + similar constructs which makes it possible to evolve the language with minimal impact to legacy code and maintaining fwd/backward compatibility.

Note also that 12e8 could also be turned into "12".`'e` (ie be turned into user defined literals, where the apostrophe would be for a deprecation time be optional for the builtin suffix subset)

@timotheecour
Copy link
Member Author

timotheecour commented Apr 1, 2021

update:

@timotheecour
Copy link
Member Author

update: github now renders number literals correctly (the latest linguist release includes github-linguist/linguist#5306 which was merged)

@metagn
Copy link
Collaborator

metagn commented Aug 22, 2023

Problem 4 was an unrelated problem fixed in #22076

Problem 3 is unsolveable and not relevant anymore after having it in 2.0

Problem 6 and 7 are RFCs (and not particularly useful ones) and since we stale RFCs now it can be independent of this issue

As listed above most things support it now including my instance of VS Code (not playground but that's playground's problem), and that's problem 8

Moving problem 2 into own issue (#22534) and closing as completed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants