ymandel created this revision.
ymandel added a reviewer: xazax.hun.
Herald added subscribers: carlosgalvezp, rnkovacs.
Herald added a reviewer: njames93.
Herald added a project: All.
ymandel requested review of this revision.
Herald added a project: clang-tools-extra.
The underlying model already supports ignoring accesses to optionals through
smart pointers. This patch exposes that option through ClangTidy's configuration
options.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D140021
Files:
clang-tools-extra/clang-tidy/bugprone/UncheckedOptionalAccessCheck.cpp
clang-tools-extra/clang-tidy/bugprone/UncheckedOptionalAccessCheck.h
clang-tools-extra/test/clang-tidy/checkers/bugprone/unchecked-optional-access-ignore-smart.cpp
Index: clang-tools-extra/test/clang-tidy/checkers/bugprone/unchecked-optional-access-ignore-smart.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone/unchecked-optional-access-ignore-smart.cpp
@@ -0,0 +1,137 @@
+// RUN: %check_clang_tidy %s bugprone-unchecked-optional-access %t -- \
+// RUN: -config="{CheckOptions: [ \
+// RUN: {key: bugprone-unchecked-optional-access.IgnoreSmartPointerDereference, value: true}]}" -- \
+// RUN: -I %S/Inputs/unchecked-optional-access
+
+#include "absl/types/optional.h"
+
+template <typename T>
+struct SmartPtr {
+ T& operator*() &;
+ T* operator->();
+};
+
+struct Bar {
+ absl::optional<int> opt;
+};
+
+void unchecked_value_access(const absl::optional<int> &opt) {
+ opt.value();
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: unchecked access to optional value [bugprone-unchecked-optional-access]
+}
+
+void unchecked_value_access_through_smart_ptr(SmartPtr<absl::optional<int>> s) {
+ s->value();
+ (*s).value();
+
+}
+
+void unchecked_value_access_through_smart_ptr_field(SmartPtr<Bar> s) {
+ s->opt.value();
+ (*s).opt.value();
+
+}
+
+void unchecked_deref_operator_access(const absl::optional<int> &opt) {
+ *opt;
+ // CHECK-MESSAGES: :[[@LINE-1]]:4: warning: unchecked access to optional value
+}
+
+struct Foo {
+ void foo() const {}
+};
+
+void unchecked_arrow_operator_access(const absl::optional<Foo> &opt) {
+ opt->foo();
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: unchecked access to optional value
+}
+
+void checked_access(const absl::optional<int> &opt) {
+ if (opt.has_value()) {
+ opt.value();
+ }
+}
+
+template <typename T>
+void function_template_without_user(const absl::optional<T> &opt) {
+ opt.value(); // no-warning
+}
+
+template <typename T>
+void function_template_with_user(const absl::optional<T> &opt) {
+ opt.value();
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: unchecked access to optional value
+}
+
+void function_template_user(const absl::optional<int> &opt) {
+ // Instantiate the f3 function template so that it gets matched by the check.
+ function_template_with_user(opt);
+}
+
+template <typename T>
+void function_template_with_specialization(const absl::optional<int> &opt) {
+ opt.value(); // no-warning
+}
+
+template <>
+void function_template_with_specialization<int>(
+ const absl::optional<int> &opt) {
+ opt.value();
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: unchecked access to optional value
+}
+
+template <typename T>
+class ClassTemplateWithSpecializations {
+ void f(const absl::optional<int> &opt) {
+ opt.value(); // no-warning
+ }
+};
+
+template <typename T>
+class ClassTemplateWithSpecializations<T *> {
+ void f(const absl::optional<int> &opt) {
+ opt.value(); // no-warning
+ }
+};
+
+template <>
+class ClassTemplateWithSpecializations<int> {
+ void f(const absl::optional<int> &opt) {
+ opt.value();
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: unchecked access to optional
+ }
+};
+
+// The templates below are not instantiated and CFGs can not be properly built
+// for them. They are here to make sure that the checker does not crash, but
+// instead ignores non-instantiated templates.
+
+template <typename T>
+struct C1 {};
+
+template <typename T>
+struct C2 : public C1<T> {
+ ~C2() {}
+};
+
+template <typename T, template <class> class B>
+struct C3 : public B<T> {
+ ~C3() {}
+};
+
+void multiple_unchecked_accesses(absl::optional<int> opt1,
+ absl::optional<int> opt2) {
+ for (int i = 0; i < 10; i++) {
+ opt1.value();
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: unchecked access to optional
+ }
+ opt2.value();
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: unchecked access to optional value
+}
+
+class C4 {
+ explicit C4(absl::optional<int> opt) : foo_(opt.value()) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:47: warning: unchecked access to optional
+ }
+ int foo_;
+};
Index: clang-tools-extra/clang-tidy/bugprone/UncheckedOptionalAccessCheck.h
===================================================================
--- clang-tools-extra/clang-tidy/bugprone/UncheckedOptionalAccessCheck.h
+++ clang-tools-extra/clang-tidy/bugprone/UncheckedOptionalAccessCheck.h
@@ -11,6 +11,7 @@
#include "../ClangTidyCheck.h"
#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.h"
namespace clang {
namespace tidy {
@@ -24,12 +25,21 @@
class UncheckedOptionalAccessCheck : public ClangTidyCheck {
public:
UncheckedOptionalAccessCheck(StringRef Name, ClangTidyContext *Context)
- : ClangTidyCheck(Name, Context) {}
+ : ClangTidyCheck(Name, Context),
+ ModelOptions{
+ Options.getLocalOrGlobal("IgnoreSmartPointerDereference", false)} {}
void registerMatchers(ast_matchers::MatchFinder *Finder) override;
void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
bool isLanguageVersionSupported(const LangOptions &LangOpts) const override {
return LangOpts.CPlusPlus;
}
+ void storeOptions(ClangTidyOptions::OptionMap &Opts) override {
+ Options.store(Opts, "IgnoreSmartPointerDereference",
+ ModelOptions.IgnoreSmartPointerDereference);
+ }
+
+private:
+ dataflow::UncheckedOptionalAccessModelOptions ModelOptions;
};
} // namespace bugprone
Index: clang-tools-extra/clang-tidy/bugprone/UncheckedOptionalAccessCheck.cpp
===================================================================
--- clang-tools-extra/clang-tidy/bugprone/UncheckedOptionalAccessCheck.cpp
+++ clang-tools-extra/clang-tidy/bugprone/UncheckedOptionalAccessCheck.cpp
@@ -9,18 +9,15 @@
#include "UncheckedOptionalAccessCheck.h"
#include "clang/AST/ASTContext.h"
#include "clang/AST/DeclCXX.h"
-#include "clang/AST/DeclTemplate.h"
#include "clang/ASTMatchers/ASTMatchFinder.h"
#include "clang/ASTMatchers/ASTMatchers.h"
#include "clang/Analysis/CFG.h"
#include "clang/Analysis/FlowSensitive/ControlFlowContext.h"
#include "clang/Analysis/FlowSensitive/DataflowAnalysisContext.h"
#include "clang/Analysis/FlowSensitive/DataflowEnvironment.h"
-#include "clang/Analysis/FlowSensitive/DataflowLattice.h"
#include "clang/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.h"
#include "clang/Analysis/FlowSensitive/WatchedLiteralsSolver.h"
#include "clang/Basic/SourceLocation.h"
-#include "llvm/ADT/Any.h"
#include "llvm/ADT/Optional.h"
#include "llvm/ADT/STLExtras.h"
#include "llvm/Support/Error.h"
@@ -33,12 +30,14 @@
using ast_matchers::MatchFinder;
using dataflow::UncheckedOptionalAccessDiagnoser;
using dataflow::UncheckedOptionalAccessModel;
+using dataflow::UncheckedOptionalAccessModelOptions;
using llvm::Optional;
static constexpr llvm::StringLiteral FuncID("fun");
static Optional<std::vector<SourceLocation>>
-analyzeFunction(const FunctionDecl &FuncDecl, ASTContext &ASTCtx) {
+analyzeFunction(const FunctionDecl &FuncDecl, ASTContext &ASTCtx,
+ UncheckedOptionalAccessModelOptions ModelOptions) {
using dataflow::ControlFlowContext;
using dataflow::DataflowAnalysisState;
using llvm::Expected;
@@ -52,7 +51,7 @@
std::make_unique<dataflow::WatchedLiteralsSolver>());
dataflow::Environment Env(AnalysisContext, FuncDecl);
UncheckedOptionalAccessModel Analysis(ASTCtx);
- UncheckedOptionalAccessDiagnoser Diagnoser;
+ UncheckedOptionalAccessDiagnoser Diagnoser(ModelOptions);
std::vector<SourceLocation> Diagnostics;
Expected<std::vector<
Optional<DataflowAnalysisState<UncheckedOptionalAccessModel::Lattice>>>>
@@ -98,7 +97,7 @@
return;
if (Optional<std::vector<SourceLocation>> Errors =
- analyzeFunction(*FuncDecl, *Result.Context))
+ analyzeFunction(*FuncDecl, *Result.Context, ModelOptions))
for (const SourceLocation &Loc : *Errors)
diag(Loc, "unchecked access to optional value");
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
