felix642 updated this revision to Diff 556326.
felix642 added a comment.
Updated Differential to truly support optional parameters
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D159436/new/
https://reviews.llvm.org/D159436
Files:
clang-tools-extra/clang-tidy/ClangTidyCheck.h
clang-tools-extra/clang-tidy/readability/FunctionSizeCheck.cpp
clang-tools-extra/clang-tidy/readability/FunctionSizeCheck.h
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/test/clang-tidy/infrastructure/optional-parameter.cpp
Index: clang-tools-extra/test/clang-tidy/infrastructure/optional-parameter.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/infrastructure/optional-parameter.cpp
@@ -0,0 +1,24 @@
+// RUN: %check_clang_tidy %s bugprone-easily-swappable-parameters %t \
+// RUN: -config='{CheckOptions: { \
+// RUN: bugprone-easily-swappable-parameters.MinimumLength: "", \
+// RUN: }}' --
+
+// RUN: %check_clang_tidy %s bugprone-easily-swappable-parameters %t \
+// RUN: -config='{CheckOptions: { \
+// RUN: bugprone-easily-swappable-parameters.MinimumLength: "none", \
+// RUN: }}' --
+
+// RUN: %check_clang_tidy %s bugprone-easily-swappable-parameters %t \
+// RUN: -config='{CheckOptions: { \
+// RUN: bugprone-easily-swappable-parameters.MinimumLength: "null", \
+// RUN: }}' --
+
+// RUN: %check_clang_tidy %s bugprone-easily-swappable-parameters %t \
+// RUN: -config='{CheckOptions: { \
+// RUN: bugprone-easily-swappable-parameters.MinimumLength: "false", \
+// RUN: }}' --
+
+void a(int b, int c) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: 2 adjacent parameters of 'a' of similar type ('int') are easily swapped by mistake [bugprone-easily-swappable-parameters]
+// CHECK-MESSAGES: :[[@LINE-2]]:12: note: the first parameter in the range is 'b'
+// CHECK-MESSAGES: :[[@LINE-3]]:19: note: the last parameter in the range is 'c'
Index: clang-tools-extra/docs/ReleaseNotes.rst
===================================================================
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -113,6 +113,10 @@
- Improved `--dump-config` to print check options in alphabetical order.
+- Added support for optional parameters. If a parameter that requires an integer
+ literal in the config file is set to `none`, `null`, `false`, or is left empty,
+ it will use its default value
+
New checks
^^^^^^^^^^
Index: clang-tools-extra/clang-tidy/readability/FunctionSizeCheck.h
===================================================================
--- clang-tools-extra/clang-tidy/readability/FunctionSizeCheck.h
+++ clang-tools-extra/clang-tidy/readability/FunctionSizeCheck.h
@@ -41,12 +41,12 @@
void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
private:
- const unsigned LineThreshold;
- const unsigned StatementThreshold;
- const unsigned BranchThreshold;
- const unsigned ParameterThreshold;
- const unsigned NestingThreshold;
- const unsigned VariableThreshold;
+ const std::optional<unsigned> LineThreshold;
+ const std::optional<unsigned> StatementThreshold;
+ const std::optional<unsigned> BranchThreshold;
+ const std::optional<unsigned> ParameterThreshold;
+ const std::optional<unsigned> NestingThreshold;
+ const std::optional<unsigned> VariableThreshold;
};
} // namespace clang::tidy::readability
Index: clang-tools-extra/clang-tidy/readability/FunctionSizeCheck.cpp
===================================================================
--- clang-tools-extra/clang-tidy/readability/FunctionSizeCheck.cpp
+++ clang-tools-extra/clang-tidy/readability/FunctionSizeCheck.cpp
@@ -126,12 +126,12 @@
FunctionSizeCheck::FunctionSizeCheck(StringRef Name, ClangTidyContext *Context)
: ClangTidyCheck(Name, Context),
- LineThreshold(Options.get("LineThreshold", -1U)),
- StatementThreshold(Options.get("StatementThreshold", 800U)),
- BranchThreshold(Options.get("BranchThreshold", -1U)),
- ParameterThreshold(Options.get("ParameterThreshold", -1U)),
- NestingThreshold(Options.get("NestingThreshold", -1U)),
- VariableThreshold(Options.get("VariableThreshold", -1U)) {}
+ LineThreshold(Options.get<unsigned>("LineThreshold")),
+ StatementThreshold(Options.get<unsigned>("StatementThreshold", 800U)),
+ BranchThreshold(Options.get<unsigned>("BranchThreshold")),
+ ParameterThreshold(Options.get<unsigned>("ParameterThreshold")),
+ NestingThreshold(Options.get<unsigned>("NestingThreshold")),
+ VariableThreshold(Options.get<unsigned>("VariableThreshold")) {}
void FunctionSizeCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
Options.store(Opts, "LineThreshold", LineThreshold);
@@ -155,7 +155,7 @@
const auto *Func = Result.Nodes.getNodeAs<FunctionDecl>("func");
FunctionASTVisitor Visitor;
- Visitor.Info.NestingThreshold = NestingThreshold;
+ Visitor.Info.NestingThreshold = NestingThreshold.value_or(-1);
Visitor.TraverseDecl(const_cast<FunctionDecl *>(Func));
auto &FI = Visitor.Info;
@@ -173,49 +173,51 @@
unsigned ActualNumberParameters = Func->getNumParams();
- if (FI.Lines > LineThreshold || FI.Statements > StatementThreshold ||
- FI.Branches > BranchThreshold ||
- ActualNumberParameters > ParameterThreshold ||
- !FI.NestingThresholders.empty() || FI.Variables > VariableThreshold) {
+ if ((LineThreshold && FI.Lines > LineThreshold) ||
+ (StatementThreshold && FI.Statements > StatementThreshold) ||
+ (BranchThreshold && FI.Branches > BranchThreshold) ||
+ (ParameterThreshold && ActualNumberParameters > ParameterThreshold) ||
+ !FI.NestingThresholders.empty() ||
+ (VariableThreshold && FI.Variables > VariableThreshold)) {
diag(Func->getLocation(),
"function %0 exceeds recommended size/complexity thresholds")
<< Func;
}
- if (FI.Lines > LineThreshold) {
+ if (LineThreshold && FI.Lines > LineThreshold) {
diag(Func->getLocation(),
"%0 lines including whitespace and comments (threshold %1)",
DiagnosticIDs::Note)
- << FI.Lines << LineThreshold;
+ << FI.Lines << LineThreshold.value();
}
- if (FI.Statements > StatementThreshold) {
+ if (StatementThreshold && FI.Statements > StatementThreshold) {
diag(Func->getLocation(), "%0 statements (threshold %1)",
DiagnosticIDs::Note)
- << FI.Statements << StatementThreshold;
+ << FI.Statements << StatementThreshold.value();
}
- if (FI.Branches > BranchThreshold) {
+ if (BranchThreshold && FI.Branches > BranchThreshold) {
diag(Func->getLocation(), "%0 branches (threshold %1)", DiagnosticIDs::Note)
- << FI.Branches << BranchThreshold;
+ << FI.Branches << BranchThreshold.value();
}
- if (ActualNumberParameters > ParameterThreshold) {
+ if (ParameterThreshold && ActualNumberParameters > ParameterThreshold) {
diag(Func->getLocation(), "%0 parameters (threshold %1)",
DiagnosticIDs::Note)
- << ActualNumberParameters << ParameterThreshold;
+ << ActualNumberParameters << ParameterThreshold.value();
}
for (const auto &CSPos : FI.NestingThresholders) {
diag(CSPos, "nesting level %0 starts here (threshold %1)",
DiagnosticIDs::Note)
- << NestingThreshold + 1 << NestingThreshold;
+ << NestingThreshold.value() + 1 << NestingThreshold.value();
}
- if (FI.Variables > VariableThreshold) {
+ if (VariableThreshold && FI.Variables > VariableThreshold) {
diag(Func->getLocation(), "%0 variables (threshold %1)",
DiagnosticIDs::Note)
- << FI.Variables << VariableThreshold;
+ << FI.Variables << VariableThreshold.value();
}
}
Index: clang-tools-extra/clang-tidy/ClangTidyCheck.h
===================================================================
--- clang-tools-extra/clang-tidy/ClangTidyCheck.h
+++ clang-tools-extra/clang-tidy/ClangTidyCheck.h
@@ -184,8 +184,8 @@
/// integral type ``T``.
///
/// Reads the option with the check-local name \p LocalName from the
- /// ``CheckOptions``. If the corresponding key is not present, return
- /// ``std::nullopt``.
+ /// ``CheckOptions``. If the corresponding key is not present or empty,
+ /// return ``std::nullopt``.
///
/// If the corresponding key can't be parsed as a ``T``, emit a
/// diagnostic and return ``std::nullopt``.
@@ -193,6 +193,9 @@
std::enable_if_t<std::is_integral_v<T>, std::optional<T>>
get(StringRef LocalName) const {
if (std::optional<StringRef> Value = get(LocalName)) {
+ if (Value == "" || Value == "none" || Value == "null" ||
+ Value == "false" || (std::is_unsigned_v<T> && Value == "-1"))
+ return std::nullopt;
T Result{};
if (!StringRef(*Value).getAsInteger(10, Result))
return Result;
@@ -238,6 +241,9 @@
return std::nullopt;
}
T Result{};
+ if (ValueOr == "" || ValueOr == "none" || ValueOr == "null" ||
+ ValueOr == "false" || (std::is_unsigned_v<T> && ValueOr == "-1"))
+ return std::nullopt;
if (!StringRef(*ValueOr).getAsInteger(10, Result))
return Result;
diagnoseBadIntegerOption(
@@ -286,8 +292,8 @@
/// enum type ``T``.
///
/// Reads the option with the check-local name \p LocalName from the
- /// ``CheckOptions``. If the corresponding key is not present, return
- /// \p Default.
+ /// ``CheckOptions``. If the corresponding key is not present or empty,
+ /// return \p Default.
///
/// If the corresponding key can't be parsed as a ``T``, emit a
/// diagnostic and return \p Default.
@@ -356,6 +362,19 @@
storeInt(Options, LocalName, Value);
}
+ /// Stores an option with the check-local name \p LocalName with
+ /// integer value \p Value to \p Options. If the value is empty
+ /// stores ``
+ template <typename T>
+ std::enable_if_t<std::is_integral_v<T>>
+ store(ClangTidyOptions::OptionMap &Options, StringRef LocalName,
+ std::optional<T> Value) const {
+ if (Value)
+ storeInt(Options, LocalName, *Value);
+ else
+ store(Options, LocalName, "none");
+ }
+
/// Stores an option with the check-local name \p LocalName as the string
/// representation of the Enum \p Value to \p Options.
///
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
