nim-lang / RFCs

A repository for your Nim proposals.
135 stars 26 forks source link

Improvements to defines #438

Closed metagn closed 2 years ago

metagn commented 2 years ago

This proposal does not change any existing behavior, so no breaking changes.

Defines currently have 2 relevant problems I can think of, please mention if you know more:

  1. They are globally scoped, meaning 1 name will only refer to 1 define (#181 is a proposed solution for this)
  2. They have a single global value, meaning they cannot reliably be set differently for different libraries (there is some discussion about how this is a limitation of -d:asyncBackend somewhere but I cannot find it)

This is a joint proposal with solutions for both of these issues, although the second one is lower priority because I don't know many real world situations where it is important and the solution is fairly difficult.

0. Use constants with define pragmas instead of defined

Firstly, the convention should be for libraries to use .booldefine constants for their options instead of defined (I believe #269 was trying to express this?). This should be pretty straightforward to adopt as const foo = defined(foo) is common.

# in library code:
when defined(foo):
  # ...

# replaced by

const foo {.booldefine.} = false

when foo:
  # ...

A good addition here would be to merge .booldefine, .intdefine, .strdefine into a single .define pragma that infers the type of the define from the default value, if possible anyway. At the very least it should recognize type annotations. Then you could also use .push define for a define section (which is already possible for bool/int/strdefine). A defines: macro could easily replicate these however.

const
  foo {.define.} = false
  bar {.define.} = 3
  baz {.define.} = "abc"
# or
const
  foo {.define.}: bool = false
  bar {.define.}: int = 3
  baz {.define.}: string = "abc"

Another possible addition is that define pragmas could take an optional parameter that allows aliasing a define from the configuration to a differently named constant, as so:

const aliasForDefine {.define: "realDefineName".} = false
# -d:realDefineName

The name conflict with the existing .define pragma should not be too much of a problem as they are used in different contexts, however in the worst case it can have another name.

1. Namespacing (supersedes #181)

Now, we can solve the scoping issue by namespacing (here expressed with dots) the define constant with another pragma for now named .defineNamespace (or just .namespace or whatever). It can just apply to all .defines in a module if used as a top level statement, or you can use .push.

{.push define, defineNamespace: "lib".}
const
  foo = false    # change with -d:lib.foo
  bar = 3        # change with -d:lib.bar=5
  baz = "abc"    # change with -d:lib.baz="def"
{.pop.}

# or

{.defineNamespace: "lib".}
{.push define.}
const
  foo = false    # change with -d:lib.foo
  bar = 3        # change with -d:lib.bar=5
  baz = "abc"    # change with -d:lib.baz="def"
{.pop.}

181 uses {.define: "lib"} to behave as {.defineNamespace: "lib", define.}, which conflicts with the earlier alias syntax which I consider to be more intuitive. defineNamespace being separate also has the benefit of being easier to push/pop, but this is not as important.

Namespaces should also be able to have names with dots in them (namespace "lib.submod" in lib.submod.foo) but it is bad style to overuse this.

Duplicate constants in separate modules for the same define should still be allowed (as is the case already I believe), this is for defines like -d:asyncBackend where a central asyncdefines module is not really easy to have.

defined(lib.foo) syntax can also still work, but it is not the same thing as the value of foo being true, which you can see in the current behavior of .booldefine.

2. Parametric modules based on define constants

This part is much lower priority and is basically WIP. I will move it to a new issue if the preceding parts are implemented. It introduces a fair bit of complexity to the language and would take a lot of time and effort to do well, plus I don't have many examples of use cases. It likely isn't sound and has mistakes, so if you can think of improvements please say so:

You can now import the `lib` from earlier as (using placeholder syntax to make it look distinct): ```nim import lib with {foo = true, bar = 5, baz = "def"} ``` A version of `lib` with this combination of defines is compiled (if the same set of defines was not compiled already) and imported. If `lib` depends on a library `otherlib` that has its own set of defines, then these are also counted in the "parameters" of `lib`, and can be overriden: ```nim import lib with {foo = true, otherlib.bar = true} ``` The following seems like a potential use case which should be possible: ```nim import lib with {foo = true, otherlib.bar = true} import otherlib with {bar = false} ``` This overrides global nim config, command line parameters, the local config, and the default values of the constants in `lib` and `otherlib` (but not the config of `lib` and `otherlib`, i.e. if they set their own define). It also does not override defines that `lib` explicitly sets for `otherlib` in its import, meaning if `lib` has `import otherlib{bar = false}`, that cannot be overriden. If defines in `lib` and `otherlib` are omitted in the import, the normal configuration will be used to determine what they should be, unless our library that depends on `lib` is imported somewhere else with these defines set to custom values. If `lib` wants to use a different value than the default value for a define in `otherlib`, it can do the following: ```nim # otherlib.nim const foo {.define.} = 1 # lib.nim const otherlibFoo {.define.} = 2 import otherlib with {foo = otherlibFoo} # ourlib.nim import lib with {otherlibFoo = 3} ``` I do not know if this would require an initial pass to find all the `{.define.}` constants in the imported modules and their dependencies. If it would, then maybe another way of defining `.define` constants might be helpful, such as in the library config, or with a `defines` section. I don't know the exact implementation of IC but theoretically this should not conflict with it. I have no idea how/if it can work with #416, especially if the supplied options can be VM-computed, or if a `define` does not have a type annotation.
Araq commented 2 years ago

Namespaced defines are fine with me. However! Namespaced or not, the approach does not scale, N different --defines create a configuration space of 2^N and already nobody, including myself, knows all the defines that the Nim core offers. It's better to create more fine-grained modules and packages that are independent of each other that don't use the feature at all. Of course, that's easier said than done.

metagn commented 2 years ago

Decided to just write a macro for this. Defines have definitely helped in a lot of my projects, but I understand that it can get messy quickly. Closing in favor of just #181, not a kneejerk reaction as I don't see much point to the RFC otherwise.

The macro I thought of is like so:

defines(prefix = "foo"):
  const
    bar = true
    baz = 3
# becomes
const
  bar = block:
    when not declared(fooBar):
      when true is bool:
        const fooBar {.booldefine.} = true
      elif true is int:
        const fooBar {.intdefine.} = true
      else:
        const fooBar {.strdefine.} = true
    fooBar
  baz = block:
    when not declared(fooBaz):
      when 3 is bool:
        const fooBaz {.booldefine.} = 3
      elif 3 is int:
        const fooBaz {.intdefine.} = 3
      else:
        const fooBaz {.strdefine.} = 3
    fooBaz

Could be made more straightforward and expansible by defining a generic fromDefine and just using strdefine, but this is enough for now. declared(fooBar) is so you can override it by using include.