Walkthrough

The WITH_SANITIZER CMake cache option was extended to include "None" as a selectable value in addition to the existing options "Memory", "Address", "Undefined", and "Thread", allowing users to disable sanitizer selection explicitly.

Changes

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

when using cmake gui if you touch WITH_SANITIZER option it presents you with list of choices without an option to opt out from using any sanitizer:

this PR adds "None" as the first option, changing this list to the following:

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 83.94%. Comparing base (fb9a6ce) to head (c016427).

@@             Coverage Diff             @@
##           develop    #2119      +/-   ##
===========================================
- Coverage    84.31%   83.94%   -0.38%     
===========================================
  Files          158      158              
  Lines        13085    13089       +4     
  Branches      3222     3225       +3     
===========================================
- Hits         11033    10987      -46     
- Misses         989     1060      +71     
+ Partials      1063     1042      -21     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

I think this should use "OFF" instead (a CMake "falsy" value), so the features report shows WITH_SANITIZER has disabled by default or when explicitly using -DWITH_SANITIZER=OFF:

--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -114,8 +114,8 @@
 Default is no prefix (empty prefix).")
 
 # Add multi-choice option
-set(WITH_SANITIZER AUTO CACHE STRING "Enable sanitizer support")
-set_property(CACHE WITH_SANITIZER PROPERTY STRINGS "None" "Memory" "Address" "Undefined" "Thread")
+set(WITH_SANITIZER OFF CACHE STRING "Enable sanitizer support")
+set_property(CACHE WITH_SANITIZER PROPERTY STRINGS "OFF" "Memory" "Address" "Undefined" "Thread")
 
 if(BASEARCH_ARM_FOUND)
     cmake_dependent_option(WITH_ARMV6 "Build with ARMv6 SIMD" ON "NOT ARCH MATCHES \"aarch64\"" OFF)

I think this should use "OFF" instead (a CMake "falsy" value), so the features report shows WITH_SANITIZER has disabled by default or when explicitly using -DWITH_SANITIZER=OFF:

I don't know cmake much, but if that's the convention then it should be changed to "OFF". The way it is now, anything will work there afaik, any non-matching string will simply work as OFF. In short, -DWITH_SANITIZER=OFF should work now

@coderabbitai what's your take?

CMake option OFF convention falsy value string property

@pps83, I agree with @benoit-pierre's suggestion to use "OFF" instead of "None". Here's why:

CMake conventions favor "OFF":

• OFF is the canonical value for disabled options in CMake
• OFF is recognized as a "falsy" value in CMake conditionals, making it consistent with how other CMake options work
• Users expect to disable features using -DWITH_SANITIZER=OFF, which is more idiomatic than -DWITH_SANITIZER=None

Practical benefits:

• Feature reports and status messages will properly show WITH_SANITIZER as disabled when set to OFF
• It aligns with how other CMake projects handle multi-choice options where one choice means "disabled"
• GUI tools and command-line usage both benefit from this consistency

Recommendation:
Use benoit-pierre's suggested change:

set(WITH_SANITIZER OFF CACHE STRING "Enable sanitizer support")
set_property(CACHE WITH_SANITIZER PROPERTY STRINGS "OFF" "Memory" "Address" "Undefined" "Thread")

This follows established CMake best practices while still providing the same functionality you intended with "None".

@pps83, as I mentioned in my previous response, I strongly recommend using "OFF" instead of "None" for the reasons outlined:

1. Convention: "OFF" is the canonical CMake value for disabled options
2. Consistency: Users expect -DWITH_SANITIZER=OFF to disable features
3. Recognition: "OFF" is automatically recognized as a falsy value in CMake conditionals

You're correct that any non-matching string currently works as "disabled," but using the explicit "OFF" value makes the intent clearer and follows established CMake best practices.

I'd suggest implementing benoit-pierre's proposed change, which will make the option more intuitive for users familiar with CMake conventions.

@pps83, I agree with @benoit-pierre's suggestion to use "OFF" instead of "None". Here's my take:

Why "OFF" is better:

1. CMake Convention: "OFF" is the canonical value for disabled options in CMake. It's recognized as a "falsy" value in conditionals (along with NO, FALSE, 0, N, IGNORE, etc.), making it semantically correct for "no sanitizer enabled."

2. User Expectations: Developers familiar with CMake expect -DWITH_SANITIZER=OFF to disable a feature. Using "OFF" aligns with how virtually every other CMake project handles optional features.

3. Feature Reports: When CMake generates configuration summaries or feature reports, "OFF" will be properly recognized and displayed as "disabled," whereas "None" is just an arbitrary string that happens to not match any sanitizer name.

4. Consistency: Other CMake options in the ecosystem use OFF/ON or OFF/specific-values patterns, not "None"/specific-values.

Addressing your point: You're absolutely right that any non-matching string currently works as "disabled" in the implementation. However, using the explicit "OFF" value:

• Makes the intent immediately clear to anyone reading the code
• Follows the principle of least surprise for users
• Makes the option self-documenting

Recommendation: Use benoit-pierre's suggested change to replace "None" with "OFF" in both the default value and the STRINGS property.

added pr: #2141