Skip to content

Conversation

@steakhal
Copy link
Contributor

@steakhal steakhal commented Jan 7, 2025

This simplifies #120239
Addresses my comment at:
#120239 (comment)

CPP-5920

@steakhal steakhal added clang Clang issues not falling into any other category clang:static analyzer labels Jan 7, 2025
@steakhal steakhal requested a review from NagyDonat January 7, 2025 10:22
@llvmbot
Copy link
Member

llvmbot commented Jan 7, 2025

@llvm/pr-subscribers-clang-static-analyzer-1

@llvm/pr-subscribers-clang

Author: Balazs Benics (steakhal)

Changes

This simplifies #120239
Addresses my comment at:
#120239 (comment)


Full diff: https://github.com/llvm/llvm-project/pull/121910.diff

3 Files Affected:

  • (modified) clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h (+12-7)
  • (modified) clang/lib/Frontend/CompilerInvocation.cpp (+1-3)
  • (modified) clang/unittests/StaticAnalyzer/Z3CrosscheckOracleTest.cpp (+1-10)
diff --git a/clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h b/clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
index 3f341ecf8c1e4f..2c970301879d24 100644
--- a/clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
+++ b/clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
@@ -126,11 +126,18 @@ enum class CTUPhase1InliningKind { None, Small, All };
 
 class PositiveAnalyzerOption {
 public:
-  PositiveAnalyzerOption() = default;
-  PositiveAnalyzerOption(const PositiveAnalyzerOption &) = default;
-  PositiveAnalyzerOption &operator=(const PositiveAnalyzerOption &) = default;
+  constexpr PositiveAnalyzerOption() = default;
+  constexpr PositiveAnalyzerOption(unsigned Value) : Value(Value) {
+    assert(Value > 0 && "only positive values are accepted");
+  }
+  constexpr PositiveAnalyzerOption(const PositiveAnalyzerOption &) = default;
+  constexpr PositiveAnalyzerOption &
+  operator=(const PositiveAnalyzerOption &Other) {
+    Value = Other.Value;
+    return *this;
+  }
 
-  static std::optional<PositiveAnalyzerOption> create(unsigned Val) {
+  static constexpr std::optional<PositiveAnalyzerOption> create(unsigned Val) {
     if (Val == 0)
       return std::nullopt;
     return PositiveAnalyzerOption{Val};
@@ -141,11 +148,9 @@ class PositiveAnalyzerOption {
       return std::nullopt;
     return PositiveAnalyzerOption::create(Parsed);
   }
-  operator unsigned() const { return Value; }
+  constexpr operator unsigned() const { return Value; }
 
 private:
-  explicit constexpr PositiveAnalyzerOption(unsigned Value) : Value(Value) {}
-
   unsigned Value = 1;
 };
 
diff --git a/clang/lib/Frontend/CompilerInvocation.cpp b/clang/lib/Frontend/CompilerInvocation.cpp
index 6e47b374d4ed86..d711df02ce9503 100644
--- a/clang/lib/Frontend/CompilerInvocation.cpp
+++ b/clang/lib/Frontend/CompilerInvocation.cpp
@@ -1274,9 +1274,7 @@ static void initOption(AnalyzerOptions::ConfigTable &Config,
     Diags->Report(diag::err_analyzer_config_invalid_input)
         << Name << "a positive";
 
-  auto Default = PositiveAnalyzerOption::create(DefaultVal);
-  assert(Default.has_value());
-  OptionField = Default.value();
+  OptionField = DefaultVal;
 }
 
 static void parseAnalyzerConfigs(AnalyzerOptions &AnOpts,
diff --git a/clang/unittests/StaticAnalyzer/Z3CrosscheckOracleTest.cpp b/clang/unittests/StaticAnalyzer/Z3CrosscheckOracleTest.cpp
index ed8627c500098a..626f5c163d17d0 100644
--- a/clang/unittests/StaticAnalyzer/Z3CrosscheckOracleTest.cpp
+++ b/clang/unittests/StaticAnalyzer/Z3CrosscheckOracleTest.cpp
@@ -27,22 +27,13 @@ static constexpr std::optional<bool> UNDEF = std::nullopt;
 static unsigned operator""_ms(unsigned long long ms) { return ms; }
 static unsigned operator""_step(unsigned long long rlimit) { return rlimit; }
 
-template <class Ret, class Arg> static Ret makeDefaultOption(Arg Value) {
-  return Value;
-}
-template <> PositiveAnalyzerOption makeDefaultOption(int Value) {
-  auto DefaultVal = PositiveAnalyzerOption::create(Value);
-  assert(DefaultVal.has_value());
-  return DefaultVal.value();
-}
-
 static const AnalyzerOptions DefaultOpts = [] {
   AnalyzerOptions Config;
 #define ANALYZER_OPTION_DEPENDS_ON_USER_MODE(TYPE, NAME, CMDFLAG, DESC,        \
                                              SHALLOW_VAL, DEEP_VAL)            \
   ANALYZER_OPTION(TYPE, NAME, CMDFLAG, DESC, DEEP_VAL)
 #define ANALYZER_OPTION(TYPE, NAME, CMDFLAG, DESC, DEFAULT_VAL)                \
-  Config.NAME = makeDefaultOption<TYPE>(DEFAULT_VAL);
+  Config.NAME = DEFAULT_VAL;
 #include "clang/StaticAnalyzer/Core/AnalyzerOptions.def"
 
   // Remember to update the tests in this file when these values change.

@steakhal
Copy link
Contributor Author

steakhal commented Jan 7, 2025

FYI @necto

Copy link
Contributor

@NagyDonat NagyDonat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, straightforward improvement.

By the way, isn't this commit an NFC change?

@steakhal steakhal changed the title [analyzer] Simplify PositiveAnalyzerOption handling [analyzer][NFC] Simplify PositiveAnalyzerOption handling Jan 7, 2025
@steakhal steakhal merged commit 5f6b714 into llvm:main Jan 7, 2025
11 checks passed
@steakhal steakhal deleted the bb/refine-PositiveAnalyzerOption branch January 7, 2025 14:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

clang:static analyzer clang Clang issues not falling into any other category

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants