Skip to content

[analyzer][NFC] Simplify PositiveAnalyzerOption handling #121910

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jan 7, 2025
Merged

[analyzer][NFC] Simplify PositiveAnalyzerOption handling #121910

merged 1 commit into from
Jan 7, 2025

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