Not all configuration macros should hold values

Many configuration macros need to be defined with a value (i.e. -D<macro>=<value>). For some this cannot be avoided (e.g. gsl_api, gsl_CONFIG_SPAN_INDEX_TYPE), but most simply switch a particular feature on or off:

gsl_CPLUSPLUS
gsl_FEATURE_WITH_CONTAINER_TO_STD, gsl_FEATURE_MAKE_SPAN_TO_STD, gsl_FEATURE_BYTE_SPAN_TO_STD
gsl_FEATURE_IMPLICIT_MACRO, gsl_FEATURE_OWNER_MACRO, gsl_FEATURE_EXPERIMENTAL_RETURN_GUARD
gsl_CONFIG_DEPRECATE_TO_LEVEL
gsl_CONFIG_NOT_NULL_EXPLICIT_CTOR, gsl_CONFIG_NOT_NULL_GET_BY_CONST_REF
gsl_CONFIG_ALLOWS_NONSTRICT_SPAN_COMPARISON, gsl_CONFIG_ALLOWS_UNCONSTRAINED_SPAN_CONTAINER_CTOR
gsl_CONFIG_CONFIRMS_COMPILATION_ERRORS

The problem with value-sensitive definitions is that different requirements cannot be merged by the build system. For example, a library might depend on the implicit macro and the Owner() macro, so it would define:

add_library(MyLib)
target_link_libraries(MyLib
    PUBLIC
        gsl::gsl-lite)
target_compile_definitions(MyLib
    PUBLIC
        gsl_FEATURE_IMPLICIT_MACRO=1
        # don't need gsl_FEATURE_OWNER_MACRO=1 this since the feature is on by default
)

A consumer of this library doesn't need the implicit and Owner() macros in her own code, so she disables these features:

add_executable(MyApp)
target_link_libraries(MyApp
    PRIVATE
        gsl::gsl-lite
        MyLib::MyLib)
target_compile_definitions(MyApp
    PRIVATE
        gsl_FEATURE_IMPLICIT_MACRO=0
        gsl_FEATURE_OWNER_MACRO=0)

CMake now calls the compiler with both -Dgsl_FEATURE_IMPLICIT_MACRO=1 and -Dgsl_FEATURE_IMPLICIT_MACRO=0, which has unspecified behavior and may or may not cause a diagnostic. If the latter definition overrides the former, this will break the headers of MyLib which rely on the macro being available. Also, gsl_FEATURE_OWNER_MACRO=0 unconditionally disables the Owner() macro, again causing problems in public headers of MyLib included by MyApp.

Proposed resolution:

Rather than gsl_CPLUSPLUS=..., the user should simply define gsl_CPLUSPLUS11, gsl_CPLUSPLUS14, gsl_CPLUSPLUS17, or gsl_CPLUSPLUS20. If more than one is defined, gsl-lite will pick the highest version available. (Edit: Sometimes it is desired to restrain gsl-lite to a particular version (e.g. #116); to support that, we would also need macros like gsl_CPLUSPLUS_MAX14.)
Replace the gsl_FEATURE_*_TO_STD with migration macros as proposed in #181.
All other features should be off by default (cf. #178), and simply defining the feature macros would switch them on, regardless of the macro value.

Hi,

In my humble opinion, configuration macros with values are superior to macros where the library makes decision based on the "definedness" of a config macro.

Please note that I have limited experience with developing libraries, so I may be wrong on some points, but at our company, we opted for valued config macros because of the following reasons.

Changing the default value

The default value of a macro (i.e. the value when the config macro is not defined) can be easily changed in later revisions. If you use #ifdef gsl_FEATURE_OWNER_MACRO, then you can't decide later that is should be on by default. (The only way to do this is to introduce a "disabler" config macro, but then you have to decide what should happen if the feature is both enabled and disabled by the separate macros, etc.)

Config macros with more than two options

When there are multiple options, value-less config macros quickly become a chore to maintain. For example, take the macros defining the behavior of GSL when a contract is violated:

#if 2 <= defined( gsl_CONFIG_CONTRACT_VIOLATION_THROWS ) + defined( gsl_CONFIG_CONTRACT_VIOLATION_TERMINATES ) + defined ( gsl_CONFIG_CONTRACT_VIOLATION_CALLS_HANDLER )
# error only one of gsl_CONFIG_CONTRACT_VIOLATION_THROWS, gsl_CONFIG_CONTRACT_VIOLATION_TERMINATES and gsl_CONFIG_CONTRACT_VIOLATION_CALLS_HANDLER may be defined.
#elif defined( gsl_CONFIG_CONTRACT_VIOLATION_THROWS )
# define        gsl_CONFIG_CONTRACT_VIOLATION_THROWS_V 1
# define        gsl_CONFIG_CONTRACT_VIOLATION_CALLS_HANDLER_V 0
#elif defined( gsl_CONFIG_CONTRACT_VIOLATION_TERMINATES )
# define        gsl_CONFIG_CONTRACT_VIOLATION_THROWS_V 0
# define        gsl_CONFIG_CONTRACT_VIOLATION_CALLS_HANDLER_V 0
#elif defined( gsl_CONFIG_CONTRACT_VIOLATION_CALLS_HANDLER )
# define        gsl_CONFIG_CONTRACT_VIOLATION_THROWS_V 0
# define        gsl_CONFIG_CONTRACT_VIOLATION_CALLS_HANDLER_V 1
#else
# define        gsl_CONFIG_CONTRACT_VIOLATION_THROWS_V 0
# define        gsl_CONFIG_CONTRACT_VIOLATION_CALLS_HANDLER_V 0
#endif

#if gsl_ELIDE_CONTRACT_EXPECTS
# define Expects( x )  /* Expects elided */
#elif gsl_CONFIG( CONTRACT_VIOLATION_THROWS_V )
# define Expects( x )  ::gsl::fail_fast_assert( (x), "GSL: Precondition failure at " __FILE__ ":" gsl_STRINGIFY(__LINE__) );
#elif gsl_CONFIG( CONTRACT_VIOLATION_CALLS_HANDLER_V )
# define Expects( x )  ::gsl::fail_fast_assert( (x), #x, "GSL: Precondition failure", __FILE__, __LINE__ );
#else
# define Expects( x )  ::gsl::fail_fast_assert( (x) )
#endif

If we had only one configuration macro, it would be much simpler:

#if ! defined( gsl_CONFIG_CONTRACT_VIOLATION_RESPONSE )
# define gsl_CONFIG_CONTRACT_VIOLATION_RESPONSE 2 // terminate
#endif

#if gsl_ELIDE_CONTRACT_EXPECTS
# define Expects( x )  /* Expects elided */
#else
# if gsl_CONFIG( CONTRACT_VIOLATION_RESPONSE )==0 // throw
#  define Expects( x )  ::gsl::fail_fast_assert( (x), "GSL: Precondition failure at " __FILE__ ":" gsl_STRINGIFY(__LINE__) );
# elif gsl_CONFIG( CONTRACT_VIOLATION_RESPONSE )==1 // use custom handler
#  define Expects( x )  ::gsl::fail_fast_assert( (x), #x, "GSL: Precondition failure", __FILE__, __LINE__ );
# elif gsl_CONFIG( CONTRACT_VIOLATION_RESPONSE )==2 // terminate
#  define Expects( x )  ::gsl::fail_fast_assert( (x) )
# elif
#  error Unknown gsl_CONFIG_CONTRACT_VIOLATION_RESPONSE value
# endif
#endif

Usage can be made a bit nicer if we define aliases for 0, 1 and 2.

Consumers may need to really disable features

In your example, I think the creator of MyApp is in the wrong: she shouldn't disable the macros because she "doesn't need the implicit and Owner() macros in her own code". In my opinion, disabler macros should be used to disable features that are forbidden for some reason (eg. target compiler cannot handle it; it contradicts company coding guidelines; it provides duplicate functionality in the codebase; etc.), not to skip parts of the library she's not using currently.

In the above listed cases, your solution can cause subtle problems: if for some reason MyApp absolutely cannot use Owner (eg. it causes subtle runtime problems), then it could be problematic that it gets silently enabled. With your solution, the creator of MyApp cannot explicitly disable the feature, while with a valued macro, they can. Unfortunately, even with valued macros the compilers do not necessarily emit a diagnostic for the "redefinition" of the config macro, but at least they have a chance. (Or you can check for contradictions from the build system.)

FYI, my choice for configuration macros holding values and #if and is inspired on the article Advanced preprocessor tips and tricks by Anders Lindgren, section #if versus #ifdef which one should we use?

Thank you both for engaging in the discussion. Let me address your points individually.

"#ifdefs don't protect you from misspelled words, #ifs do" (from Anders Lindgren's article): Neither of MSVC, GCC, Clang, ICC issue a diagnostic when using an #if with an undefined macro (cf. https://gcc.godbolt.org/z/esyWqA), they all assume it has value 0 instead, which is conforming behavior as the article points out. Also, even if they did issue a warning we wouldn't benefit from it because we explicitly set default values for most configuration options, e.g.:
```
#ifndef  gsl_CONFIG_NOT_NULL_EXPLICIT_CTOR
# define gsl_CONFIG_NOT_NULL_EXPLICIT_CTOR  0
#endif
```
It is true that value-holding configuration macros make it much easier to change the default value. Changing defaults for value-less macros is still possible, e.g. if gsl_FEATURE_FOO was an option and now becomes the default, we can add gsl_FEATURE_NO_FOO for reverting the change.
Value-holding configuration macros lead to code that is prettier and easier to understand; I don't dispute this. However, note that we can also keep our code pretty by defining valued macros internally (e.g. gsl_FEATURE_FOO_V) but have them controlled by value-less macros.
I arguably chose a bad example to illustrate my point.

Given that they are prettier and easier to maintain, we can agree that we should try to stick with value-holding macros if they don't cause problems.

However, gsl-lite is a library, and configuration options for libraries may be transitively carried into other projects by build systems such as CMake. This can cause problems with competing options which are hard to diagnose.

Build systems don't know how to coalesce options with different values. Please allow me to illustrate this with a macro-less example. Assume we have the following:

add_library(Foo INTERFACE)
target_compile_options(Foo INTERFACE /std:c++17) # never mind the compiler-specific flag
add_library(Bar INTERFACE)
target_compile_options(Bar INTERFACE /std:c++14)
add_executable(Baz)
target_link_libraries(Baz PRIVATE Foo Bar)

Clearly we want to compile Baz with the highest language setting, i.e. /std:c++17. But CMake doesn't understand the semantics of raw compile options; it will just emit both. MSVC will then issue a warning and take the latter option, thus causing compile errors for C++17 code in Foo's headers.

However, we can instead express our desire in a way CMake will understand:

add_library(Foo INTERFACE)
target_compile_features(Foo INTERFACE cxx_std_17)
add_library(Bar INTERFACE)
target_compile_features(Bar INTERFACE cxx_std_14)
add_executable(Baz)
target_link_libraries(Baz PRIVATE Foo Bar)

CMake understands that language features are additive, and hence compiles Baz with cxx_std_17.

The situation with value-holding and value-less configuration macros is similar. CMake will simply concatenate mismatching configuration macros, and the example at https://gcc.godbolt.org/z/9-Q0Kf demonstrates that not all compilers issue a warning if the same macro is defined with multiple contradicting values, so this can easily go unnoticed. The discussion in #121 explains why e.g. introducing a value-holding gsl_CONFIG_SELECT_SPAN macro would be a recipe for disaster for exactly this reason.

With value-less configuration macros, we can let the build system accumulate all transitive flags and implement the desired semantics ourselves. For example, we can make different options mutually exclusive:

#if 2 <= defined( gsl_CONFIG_CONTRACT_VIOLATION_THROWS ) + defined( gsl_CONFIG_CONTRACT_VIOLATION_TERMINATES ) + defined ( gsl_CONFIG_CONTRACT_VIOLATION_CALLS_HANDLER )
# error only one of gsl_CONFIG_CONTRACT_VIOLATION_THROWS, gsl_CONFIG_CONTRACT_VIOLATION_TERMINATES and gsl_CONFIG_CONTRACT_VIOLATION_CALLS_HANDLER may be defined.
#elif ...

Or we can implement additive semantics, e.g. if we have macros gsl_CPLUSPLUS11, gsl_CPLUSPLUS14 etc.:

#if defined( gsl_CPLUSPLUS20 )
# define gsl_CPLUSPLUS 202000L // doesn't matter if gsl_CPLUSPLUS17 is also defined
#elif defined( gsl_CPLUSPLUS17 )
# define gsl_CPLUSPLUS 201703L
#elif ...

We can also combine both semantics, e.g. if we have competing gsl_CPLUSPLUS_MAX17, gsl_CPLUSPLUS_MAX14, gsl_CPLUSPLUS_MAX11 macros:

#if defined( gsl_CPLUSPLUS_MAX17 )
# define gsl_CPLUSPLUS_MAX 201703L
#elif defined( gsl_CPLUSPLUS_MAX14 )
# define gsl_CPLUSPLUS_MAX 201402L 
#elif ...
#endif

#if gsl_CPLUSPLUS > gsl_CPLUSPLUS_MAX
# error Conflicting minimum and maximum language requirements
#endif

Whether these checks are useful is a different question, and probably best answered individually for every macro. I slightly changed the title of the issue to acknowledge this.

The first question regarding any configuration macro should be, can this macro reasonably appear in the INTERFACE part of a library? The following should not:

gsl_CONFIG_CONFIRMS_COMPILATION_ERRORS, which only affects the test suite.
gsl_CPLUSPLUS, which can be used to deal with intractable compilers; users would probably set this in global build flags such as the CXXFLAGS environment variable.
gsl_CONFIG_CONTRACT_LEVEL_*, gsl_CONFIG_CONTRACT_EXPECTS_ONLY/gsl_CONFIG_CONTRACT_ENSURES_ONLY, gsl_CONFIG_CONTRACT_VIOLATION_*: These should only ever be applied as PRIVATE flags. It is kind of acceptable¹ to compile a library e.g. with gsl_CONFIG_CONTRACT_LEVEL_ASSUME (so there are no checks in separately compiled library code) but to define gsl_CONFIG_CONTRACT_LEVEL_ON in the consuming application. A library should never impose a contract checking mode on its consumers, hence these macros should never be propagated through a target's INTERFACE.
gsl_FEATURE_*_TO_STD and the proposed gsl_MIGRATE_CXX<version>[_API] macros (cf. #181): These macros were designed to aid migration; transitively applying them to dependent projects is not useful.

All these macros could be value-holding configuration macros. But having value-less macros for contract-related settings is a slight benefit because we get a compiler error if we erroneously define a contract setting in an INTERFACE:

add_library(Foo STATIC)
target_compile_definitions(Foo PUBLIC gsl_CONFIG_CONTRACT_LEVEL_ASSUME) # bug, should be `PRIVATE`
add_library(FooTest)
target_compile_options(FooTest PRIVATE gsl_CONFIG_CONTRACT_LEVEL_AUDIT gsl_CONFIG_CONTRACT_VIOLATION_THROWS)
target_link_libraries(FooTest PRIVATE Foo) # error because both *_ASSUME and *_AUDIT are defined

But now for the hard part.

gsl_api and gsl_CONFIG_SPAN_INDEX_TYPE must be defined transitively, otherwise linking errors may ensue. Libraries probably shouldn't set either to avoid forcing them upon their users. But if they do, we'd rather get a proper diagnostic ("conflicting values for gsl_CONFIG_SPAN_INDEX_TYPE"), which is only possible with value-less macros (e.g. gsl_CONFIG_SPAN_SIGNED_INDEX and gsl_CONFIG_SPAN_UNSIGNED_INDEX).
gsl_FEATURE_EXPERIMENTAL_RETURN_GUARD, gsl_CONFIG_ALLOWS_NONSTRICT_SPAN_COMPARISON: these are additive and don't break non-pathological code, so libraries may transitively enable them, but there is no need to transitively disable them. If these were value-less (and had *_NO_* counterparts for suppression), we could get a reliable notification if an app tries to suppress either feature in conflict with a library that relies on it.
gsl_FEATURE_IMPLICIT_MACRO, gsl_FEATURE_OWNER_MACRO: these change whether certain macros are defined; a library may want to enable or disable them transitively (if it uses the feature, or if the feature conflicts with code in the library's header). We want to be notified if conflicts occur, hence value-less would be preferable.
gsl_CONFIG_ALLOWS_UNCONSTRAINED_SPAN_CONTAINER_CTOR: enabling or disabling this can break code. We want to be notified if conflicts occur, hence value-less would be preferable.
gsl_CONFIG_NOT_NULL_EXPLICIT_CTOR: enabling this can break code. We want to be notified if conflicts occur, hence value-less would be preferable.
gsl_CONFIG_NOT_NULL_GET_BY_CONST_REF: disabling this can break code. We want to be notified if conflicts occur, hence value-less would be preferable.

1: ODR violations are possible but usually without grave consequences (though I wish we didn't have gsl_noexcept).

In conjunction with #179, I could imagine to arrive in a state where applying any configuration macro, except for purely additive ones such as gsl_FEATURE_EXPERIMENTAL_RETURN_GUARD and gsl_CONFIG_ALLOWS_NONSTRICT_SPAN_COMPARISON, for contract-related macros, and for gsl_MIGRATE_CXX*, would lead to a warning that says "You have applied a configuration macro that causes problems if applied transitively. Please don't do this if your project is a library, otherwise you force this configuration setting upon all of your users. Define gsl_ACKNOWLEDGE_NONSTANDARD_CONFIG to make this warning go away."

I'm closing this because #272 added extensive safety checks for configuration macros, both value-holding and value-less. I currently don't plan to make incompatible changes to configuration macros, so the issues with conflicting transitive configuration settings probably cannot be addressed.

gsl-lite / gsl-lite