lordgamez commented on code in PR #2174: URL: https://github.com/apache/nifi-minifi-cpp/pull/2174#discussion_r3281817792
########## CMakeLists.txt: ########## @@ -302,7 +301,14 @@ include(GslLite) # Add necessary definitions based on the value of STRICT_GSL_CHECKS, see gsl-lite README for more details list(APPEND GslDefinitions gsl_CONFIG_DEFAULTS_VERSION=1) -list(APPEND GslDefinitionsNonStrict gsl_CONFIG_CONTRACT_VIOLATION_THROWS gsl_CONFIG_NARROW_THROWS_ON_TRUNCATION=1) +list(APPEND GslDefinitions gsl_FEATURE_BYTE=1) +list(APPEND GslDefinitions gsl_CONFIG_NARROW_THROWS_ON_TRUNCATION=1) Review Comment: In https://github.com/apache/nifi-minifi-cpp/pull/2174/commits/02aac2d186f9d1c460d2bca9453073c3fd3a1524 I added `gsl_CONFIG_NARROW_THROWS_ON_TRUNCATION=0` to `GslDefinitionsNonStrict` and removed `gsl_CONFIG_DEFAULTS_VERSION=1` as it is already the default. For other options that have changed their defaults since the previously used version: - The new defaults of `gsl_FEATURE_STRING_SPAN` and `gsl_CONFIG_ALLOWS_SPAN_COMPARISON` are consistent with C++20 `std::span` so we can leave it at that - `gsl_FEATURE_BYTE`: we already set it to 1 manually - `gsl_CONFIG_DEPRECATE_TO_LEVEL`: default changed to 9 which is good to have the highest deprecation level on by default - `gsl_CONFIG_INDEX_TYPE`: defaults to a more domain specific index type which we do not conflict with so we can leave it at that - `gsl_CONFIG_NOT_NULL_EXPLICIT_CTOR`: default 1 is used for explicit ctor due to https://github.com/Microsoft/GSL/issues/395 so the default makes sense, it is only suggested to change it as a workaround if the legacy code conflicts with it - `gsl_CONFIG_TRANSPARENT_NOT_NULL`: the default is 1 using a transparent not_null type which conforms with our usage - default runtime contract violation handling: new default is `gsl_CONFIG_CONTRACT_VIOLATION_ASSERTS` instead of `gsl_CONFIG_CONTRACT_VIOLATION_TERMINATES` which is a subtle change calling `std::abort` instead of `std::terminate` but with more information in case of a crash. Reason given by gsl-lite: the mode enabled by `gsl_CONFIG_CONTRACT_VIOLATION_ASSERTS` is consistent with the behavior of the `assert()` macro while retaining runtime contract checks even if `NDEBUG` is defined. With this explanation I think we can keep that as well on default setting. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
