adukeman created this revision.
adukeman added reviewers: njames93, ymandel, sgatev.
Herald added subscribers: PiotrZSL, carlosgalvezp, xazax.hun.
Herald added a reviewer: NoQ.
Herald added a project: All.
adukeman requested review of this revision.
Herald added projects: clang, clang-tools-extra.
Herald added a subscriber: cfe-commits.
The unchecked-optional-access check identifies attempted value
unwrapping without checking if the value exists. These changes extend
that support to checking folly::Optional.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D155890
Files:
clang-tools-extra/test/clang-tidy/checkers/bugprone/Inputs/unchecked-optional-access/folly/types/Optional.h
clang-tools-extra/test/clang-tidy/checkers/bugprone/unchecked-optional-access.cpp
clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
Index: clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
===================================================================
--- clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
+++ clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
@@ -56,9 +56,10 @@
}
if (RD.getName() == "Optional") {
- // Check whether namespace is "::base".
+ // Check whether namespace is "::base" or "::folly".
const auto *N = dyn_cast_or_null<NamespaceDecl>(RD.getDeclContext());
- return N != nullptr && isTopLevelNamespaceWithName(*N, "base");
+ return N != nullptr && (isTopLevelNamespaceWithName(*N, "base") ||
+ isTopLevelNamespaceWithName(*N, "folly"));
}
return false;
@@ -109,15 +110,15 @@
}
auto isMakeOptionalCall() {
- return callExpr(
- callee(functionDecl(hasAnyName(
- "std::make_optional", "base::make_optional", "absl::make_optional"))),
- hasOptionalType());
+ return callExpr(callee(functionDecl(hasAnyName(
+ "std::make_optional", "base::make_optional",
+ "absl::make_optional", "folly::make_optional"))),
+ hasOptionalType());
}
auto nulloptTypeDecl() {
- return namedDecl(
- hasAnyName("std::nullopt_t", "absl::nullopt_t", "base::nullopt_t"));
+ return namedDecl(hasAnyName("std::nullopt_t", "absl::nullopt_t",
+ "base::nullopt_t", "folly::None"));
}
auto hasNulloptType() { return hasType(nulloptTypeDecl()); }
@@ -129,8 +130,8 @@
}
auto inPlaceClass() {
- return recordDecl(
- hasAnyName("std::in_place_t", "absl::in_place_t", "base::in_place_t"));
+ return recordDecl(hasAnyName("std::in_place_t", "absl::in_place_t",
+ "base::in_place_t", "folly::in_place_t"));
}
auto isOptionalNulloptConstructor() {
@@ -839,6 +840,11 @@
isOptionalMemberCallWithName("has_value"),
transferOptionalHasValueCall)
+ // optional::hasValue for folly::Optional
+ .CaseOfCFGStmt<CXXMemberCallExpr>(
+ isOptionalMemberCallWithName("hasValue"),
+ transferOptionalHasValueCall)
+
// optional::operator bool
.CaseOfCFGStmt<CXXMemberCallExpr>(
isOptionalMemberCallWithName("operator bool"),
Index: clang-tools-extra/test/clang-tidy/checkers/bugprone/unchecked-optional-access.cpp
===================================================================
--- clang-tools-extra/test/clang-tidy/checkers/bugprone/unchecked-optional-access.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone/unchecked-optional-access.cpp
@@ -1,6 +1,7 @@
// RUN: %check_clang_tidy %s bugprone-unchecked-optional-access %t -- -- -I %S/Inputs/unchecked-optional-access
#include "absl/types/optional.h"
+#include "folly/types/Optional.h"
void unchecked_value_access(const absl::optional<int> &opt) {
opt.value();
@@ -21,12 +22,34 @@
// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: unchecked access to optional value
}
+void folly_check_value_then_reset(folly::Optional<int> opt) {
+ if (opt) {
+ opt.reset();
+ opt.value();
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: unchecked access to optional value
+ }
+}
+
+void folly_value_after_swap(folly::Optional<int> opt1, folly::Optional<int> opt2) {
+ if (opt1) {
+ opt1.swap(opt2);
+ opt1.value();
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: unchecked access to optional value
+ }
+}
+
void checked_access(const absl::optional<int> &opt) {
if (opt.has_value()) {
opt.value();
}
}
+void folly_checked_access(const folly::Optional<int> &opt) {
+ if (opt.hasValue()) {
+ opt.value();
+ }
+}
+
template <typename T>
void function_template_without_user(const absl::optional<T> &opt) {
opt.value(); // no-warning
Index: clang-tools-extra/test/clang-tidy/checkers/bugprone/Inputs/unchecked-optional-access/folly/types/Optional.h
===================================================================
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone/Inputs/unchecked-optional-access/folly/types/Optional.h
@@ -0,0 +1,56 @@
+#ifndef LLVM_CLANG_TOOLS_EXTRA_TEST_CLANG_TIDY_CHECKERS_BUGPRONE_INPUTS_UNCHECKED_OPTIONAL_ACCESS_FOLLY_TYPES_OPTIONAL_H_
+#define LLVM_CLANG_TOOLS_EXTRA_TEST_CLANG_TIDY_CHECKERS_BUGPRONE_INPUTS_UNCHECKED_OPTIONAL_ACCESS_FOLLY_TYPES_OPTIONAL_H_
+
+/// Mock of `folly::Optional`.
+namespace folly {
+
+struct None {
+ constexpr explicit None() {}
+};
+
+constexpr None none;
+
+template <typename T>
+class Optional {
+public:
+ constexpr Optional() noexcept;
+
+ constexpr Optional(None) noexcept;
+
+ Optional(const Optional &) = default;
+
+ Optional(Optional &&) = default;
+
+ const T &operator*() const &;
+ T &operator*() &;
+ const T &&operator*() const &&;
+ T &&operator*() &&;
+
+ const T *operator->() const;
+ T *operator->();
+
+ const T &value() const &;
+ T &value() &;
+ const T &&value() const &&;
+ T &&value() &&;
+
+ constexpr explicit operator bool() const noexcept;
+ constexpr bool has_value() const noexcept;
+ constexpr bool hasValue() const noexcept;
+
+ template <typename U>
+ constexpr T value_or(U &&v) const &;
+ template <typename U>
+ T value_or(U &&v) &&;
+
+ template <typename... Args>
+ T &emplace(Args &&...args);
+
+ void reset() noexcept;
+
+ void swap(Optional &rhs) noexcept;
+};
+
+} // namespace folly
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_TEST_CLANG_TIDY_CHECKERS_BUGPRONE_INPUTS_UNCHECKED_OPTIONAL_ACCESS_FOLLY_TYPES_OPTIONAL_H_
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
