-
Notifications
You must be signed in to change notification settings - Fork 23
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
Improvements to defines #438
Comments
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. |
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 |
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:
-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 ofdefined
(I believe #269 was trying to express this?). This should be pretty straightforward to adopt asconst foo = defined(foo)
is common.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). Adefines:
macro could easily replicate these however.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:
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.define
s in a module if used as a top level statement, or you can use.push
.#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 centralasyncdefines
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 offoo
beingtrue
, 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):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 libraryotherlib
that has its own set of defines, then these are also counted in the "parameters" oflib
, and can be overriden:The following seems like a potential use case which should be possible:
This overrides global nim config, command line parameters, the local config, and the default values of the constants in
lib
andotherlib
(but not the config oflib
andotherlib
, i.e. if they set their own define). It also does not override defines thatlib
explicitly sets forotherlib
in its import, meaning iflib
hasimport otherlib{bar = false}
, that cannot be overriden.If defines in
lib
andotherlib
are omitted in the import, the normal configuration will be used to determine what they should be, unless our library that depends onlib
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 inotherlib
, it can do the following: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 adefines
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.The text was updated successfully, but these errors were encountered: