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

make proc nimQuery(setting: Setting): RootObj {.magic.} => return a type that depends on the enum #9

Open
timotheecour opened this issue Jun 2, 2020 · 5 comments

Comments

@timotheecour
Copy link
Member

timotheecour commented Jun 2, 2020

@Araq ok with following?

proposal

  • deprecate compileOption, querySettingSeq and querySetting (or "soft" deprecate ie recommend the alternative here)
  • introduce
# in compilesettings
proc nimQuery*(setting: Setting): RootObj {.compileTime, magic.}

why RootObj ? because that's what used in iterator fields*[T: tuple|object](x: T): RootObj to allow returning a type that can depend on input argument

example

doAssert nimQuery(nimVerCT) == (1,3,7)
  # I need this in many places, after this we don't need to define new symbols in condsyms anymore since we can test for nimVerCT
  # i had originally introduced it in https://github.com/nim-lang/Nim/pull/14447#issuecomment-633725338 but moved it out of there
doAssert nimQuery(assertions) # can replace `compileOption("assertions")`
doAssert nimQuery(backend) == Backend.js
doAssert nimQuery(arguments) is seq[string]
doAssert Experimental.notnil in nimQuery(experimental)

note

compilesettings stays a "low-level" module in the sense it's usable by low level modules (eg doesn't depend on iterators, dollars etc)

implementation

no particular difficulty

@Araq
Copy link
Member

Araq commented Jun 2, 2020

Oh my, please ... no. I think it's a bad design, reminds me of sysctl. It's faking simplicity.

@timotheecour
Copy link
Member Author

timotheecour commented Jun 2, 2020

i was half expecting that, and I kind of agree. Then this?

proc nimVerCT*(): tuple(int,int,int)
proc getExperimental*(): seq[Experimental] # type Experimental = enum ...
proc getBackend(): Backend
when nimVerCT() < (1,3,5) : ...
when Experimental.notnil in getExperimental(): ...

@Araq
Copy link
Member

Araq commented Jun 2, 2020

What's wrong with the existing solutions?

@timotheecour
Copy link
Member Author

timotheecour commented Jun 2, 2020

  • Hungarian-ish notation is redundant (querySetting, querySettingSeq, querySettingBool, ...)

  • like you've said yourself, stringly typed API's ("everything is a string / seq[string]") aren't user friendly in a static language that makes things like enum etc trivial to use

# we shouldn't need serialization + deserialization
doAssert querySetting("nimverct").parseTo((int,int,int)) == (1,3,5)
doAssert querySetting("assertions").parseTo(bool) # or reach for `system.compileOption` ... but why

and the compiler can't always help you if you misuse:

doAssert "forLoopmacros" in querySetting("experimental")  # bug (wrong style => you'll silently get wrong answer)
doAssert "forLoopMacros".normalize in querySetting("experimental") # still buggy if you mistype

This is more user friendly, self documenting and impossible to misuse or you'll get CT error:

# either the "sysctl"-like way i gave in top post, or as individual procs, I'm agnostic:
doAssert nimVerCT() == (1,3,5)
doAssert Experimental.forLoopMacros in getExperimental()
doAssert assertions() # or maybe `nimQuery(assertions)` still in this case
  • a 2ndary goal is to consolidate these in compilesettings so you don't have some arbitrary set of flags in system.compileOption and some other arbitrary set of flags in compilesettings.{querySetting,querySettingSeq}

@Araq
Copy link
Member

Araq commented Jun 3, 2020

Well as long as it doesn't bloat system.nim further, I'm fine with more type-safe additions.

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

2 participants