Improve compile-time flags

[ysbaddaden]

I miss a couple features related to compile time flags in the compiler:

1. flags with a value

   It can be emulated with -Dkey=value then checking for full "key=value" flags, for example {% if flag?("evloop=epoll") %}, it's a bit clunky but it works... until we need to manipulate the value(s), for example -Dbranch-protection=bti,pac-ret,leaf where we'd have to split on the comma to enable this or that feature in that specific order.

   For this we need support from the compiler. For example a Crystal::Program#flag?(name) method that would in order: check if a value-less flag exists, check if a value flag exists and return its value (string) or return false. Then we could update the flag? macro method to use it instead of Crystal::Program#has_flag?.

   I don't see any backward compatibility issues?

2. defining flags from macros

   Sometimes it would be useful to define a flag from macros, for example to set some default flags on each platform, right from the stdlib code, instead of hardcoding those in the compiler or having to duplicate the target checks in many places...

   There could for example be a pair of set_flag(name) and set_flag(name, value) macro methods.

[straight-shoota]

For retrieveing a value we have two options:

• Integrate into the existing flag? macro method. Maybe a trailing = could indicate that we want to get the value of the flag, not check its existence: {{ flag?("branch-protection=") }} # => "bti,pac-ret,leaf". This would change the return type of flag? to BoolLiteral | StringLiteral. I don't see any issues with backward compatibility, but it might be piling a bit much on a single macro method.
• Add a new macro method flag_value (or flag_option or other). This would be a separate feature, clearly distinct from the flag? macro: {{ flag_value("branch-protection") }} # => "bti,pac-ret,leaf". The return type would be StringLiteral | NilLiteral.

We also need to consider what happens when a flag key is passed multiple times: -Dbranch-protection=bti Dbranch-protection=pac-ret. What's the value of that? Does only the last one count? Or do we merge them together, as StringLiteral or ArrayLiteral? I'm not sure about concrete use cases, but it might be realistic that you might want to accept multiple values via individual assignments 🤷
This isn't relevant for flag? because setting a flag multiple times has the same effect as setting it once.

[straight-shoota]

I'm not sure about how useful defining flags from use code would be. A practical concern would be that this means flags change during the compilation process which can lead to unexpected behaviour.

We might need to look a bit into use cases to find a good motivation for this. But I suppose we can always emulate a similar effect by checking the definition of constants (as we did in #14996).

IMO flags derived from the target triple fit perfectly into the compiler. I don't see any benefit of moving them to code. It'll reduce performance (macros are quite fast, but native code is faster).

[ysbaddaden]

I was thinking more in terms of runtime features that may depend on some flag, for which we'd like to define default values depending on the target... or not (e.g. always enable a feature in a shard).

The only way to achieve that is to hardcode them in the compiler (and the default be unavailable until the next release and previous releases wouldn't work), while a set_flag macro method would allow apps and shards to enable features right in code (no need for cli flags):

# always enable feature X because we need it
{% set_flag("mylib_enable_x") %}
require "mylib"

Checking on constants works but it feels clunky (we may not have different constants) and setting a flag should be much faster than injecting a constant just for enabling some feature.

[ysbaddaden]

A 3rd feature would be to define compile time flags somewhere and have them applied by crystal automatically (no need for manual -D command line arguments). For example a flags: list in shard.yml.

I.e. this is reminiscent of crystal editions.

I was thinking about it as a counter argument to the set flag macro a bit, but I still find value in setting flags based upon other flags purely in code.

[straight-shoota]

Yeah flags for runtime features should not be defined by the compiler 👍

I think it's hard to reason about the benefits of different approaches without concrete use cases.

For flag values we have a concrete use case, passing multiple branch-protection options.
But even that isn't really critical. It's already possible to use a couple of individual flags -Dbranch-protection=bti -Dbranch-protection=pac-ret -Dbranch-protection=leaf instead of a combined -Dbranch-protection=bti,pac-ret,leaf. It's a bit more verbose, but that's it. No need for a new compiler feature.

An explicit way to retrieve flag values is only really necessary when the possible values are unknown.
That would be a similar use case as environment flags, which we're already using for some compiler configuration (CRYSTAL_CONFIG_CC, CRYSTAL_CONFIG_PATH, CRYSTAL_CONFIG_TARGET etc.). But I don't think it makes much sense to turn them into compiler flags.