github-actions[bot] wrote:
<!--LLVM CODE FORMAT COMMENT: {clang-format}-->
:warning: C/C++ code formatter, clang-format found issues in your code.
:warning:
<details>
<summary>
You can test this locally with the following command:
</summary>
``````````bash
git-clang-format --diff origin/main HEAD --extensions cpp,h --
hicketts/hicketts_optional.h hicketts/isValidAnalyzeAsMethodAttr.cpp
hicketts/test_hicketts_optional.cpp
clang-tools-extra/test/clang-tidy/checkers/bugprone/unchecked-optional-access.cpp
clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
clang/lib/Sema/SemaDeclAttr.cpp
clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
--diff_from_common_commit
``````````
:warning:
The reproduction instructions above might return results for more than one PR
in a stack if you are using a stacked PR workflow. You can limit the results by
changing `origin/main` to the base branch/commit you want to compare against.
:warning:
</details>
<details>
<summary>
View the diff from clang-format here.
</summary>
``````````diff
diff --git
a/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
b/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
index 0f448a60d..021b5697c 100644
--- a/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
+++ b/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
@@ -12,8 +12,8 @@
//===----------------------------------------------------------------------===//
#include "clang/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.h"
-#include "clang/AST/Attr.h"
#include "clang/AST/ASTContext.h"
+#include "clang/AST/Attr.h"
#include "clang/AST/DeclCXX.h"
#include "clang/AST/Expr.h"
#include "clang/AST/ExprCXX.h"
@@ -362,10 +362,12 @@ auto isValueOrStringEmptyCall() {
return cxxMemberCallExpr(
callee(cxxMethodDecl(hasName("empty"))),
onImplicitObjectArgument(ignoringImplicit(
- cxxMemberCallExpr(on(expr(unless(cxxThisExpr()))),
- callee(cxxMethodDecl(anyOf(hasName("value_or"),
hasAnalyzeAsMethodName("value_or")),
- ofClass(optionalClass()))),
- hasArgument(0, stringLiteral(hasSize(0))))
+ cxxMemberCallExpr(
+ on(expr(unless(cxxThisExpr()))),
+ callee(cxxMethodDecl(anyOf(hasName("value_or"),
+ hasAnalyzeAsMethodName("value_or")),
+ ofClass(optionalClass()))),
+ hasArgument(0, stringLiteral(hasSize(0))))
.bind(ValueOrCallID))));
}
@@ -1079,7 +1081,7 @@ auto buildTransferMatchSwitch() {
.CaseOfCFGStmt<CXXMemberCallExpr>(
isOptionalMemberCallWithNameMatcher(
anyOf(hasAnyName("has_value", "hasValue"),
- hasAnalyzeAsMethodName("has_value"))),
+ hasAnalyzeAsMethodName("has_value"))),
transferOptionalHasValueCall)
// optional::operator bool
@@ -1088,18 +1090,17 @@ auto buildTransferMatchSwitch() {
transferOptionalHasValueCall)
// this code could be removed if NullableValue used
- // [[clang::analyze_as_inverse_method("std::optional::has_value")]] on
isNull() *NYI
- // NullableValue::isNull
- // Only NullableValue has isNull
+ // [[clang::analyze_as_inverse_method("std::optional::has_value")]] on
+ // isNull() *NYI NullableValue::isNull Only NullableValue has isNull
.CaseOfCFGStmt<CXXMemberCallExpr>(
isOptionalMemberCallWithNameMatcher(hasName("isNull")),
transferOptionalIsNullCall)
// this code could be removed if NullableValue used
- // [[clang::analyze_as_method("emplace")]] on makeValue() and
makeValueInplace()
- // NullableValue::makeValue, NullableValue::makeValueInplace
- // Only NullableValue has these methods, but this
- // will also pass for other types
+ // [[clang::analyze_as_method("emplace")]] on makeValue() and
+ // makeValueInplace() NullableValue::makeValue,
+ // NullableValue::makeValueInplace Only NullableValue has these methods,
+ // but this will also pass for other types
.CaseOfCFGStmt<CXXMemberCallExpr>(
isOptionalMemberCallWithNameMatcher(
hasAnyName("makeValue", "makeValueInplace")),
@@ -1113,7 +1114,8 @@ auto buildTransferMatchSwitch() {
// optional::emplace
.CaseOfCFGStmt<CXXMemberCallExpr>(
- isOptionalMemberCallWithNameMatcher(anyOf(hasName("emplace"),
hasAnalyzeAsMethodName("emplace"))),
+ isOptionalMemberCallWithNameMatcher(
+ anyOf(hasName("emplace"), hasAnalyzeAsMethodName("emplace"))),
[](const CXXMemberCallExpr *E, const MatchFinder::MatchResult &,
LatticeTransferState &State) {
if (RecordStorageLocation *Loc =
@@ -1124,7 +1126,8 @@ auto buildTransferMatchSwitch() {
// optional::reset
.CaseOfCFGStmt<CXXMemberCallExpr>(
- isOptionalMemberCallWithNameMatcher(anyOf(hasName("reset"),
hasAnalyzeAsMethodName("reset"))),
+ isOptionalMemberCallWithNameMatcher(
+ anyOf(hasName("reset"), hasAnalyzeAsMethodName("reset"))),
[](const CXXMemberCallExpr *E, const MatchFinder::MatchResult &,
LatticeTransferState &State) {
if (RecordStorageLocation *Loc =
@@ -1136,7 +1139,8 @@ auto buildTransferMatchSwitch() {
// optional::swap
.CaseOfCFGStmt<CXXMemberCallExpr>(
- isOptionalMemberCallWithNameMatcher(anyOf(hasName("swap"),
hasAnalyzeAsMethodName("swap"))),
+ isOptionalMemberCallWithNameMatcher(
+ anyOf(hasName("swap"), hasAnalyzeAsMethodName("swap"))),
transferSwapCall)
// std::swap
diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp
index 2d5110071..572cef4fc 100644
--- a/clang/lib/Sema/SemaDeclAttr.cpp
+++ b/clang/lib/Sema/SemaDeclAttr.cpp
@@ -6663,29 +6663,29 @@ static void handleAnalyzeAsClass(Sema &S, Decl *D,
const ParsedAttr &AL) {
// for now this only handles std::optional (POC)
static bool isValidAnalyzeAsMethodAttr(Decl *D, StringRef Tag) {
- // no validation is done currently. if someone writes something with a
nonsense name,
- // it simply won't be validated but also no warning will be emitted
- // would be nice to do something smarter in the real implementation
+ // no validation is done currently. if someone writes something with a
+ // nonsense name, it simply won't be validated but also no warning will be
+ // emitted would be nice to do something smarter in the real implementation
- if(Tag.empty())
+ if (Tag.empty())
return false;
size_t OpenParen = Tag.find('(');
- if(OpenParen == StringRef::npos)
- return true; // simple function name with no arguments
+ if (OpenParen == StringRef::npos)
+ return true; // simple function name with no arguments
- if(OpenParen == 0)
- return false; // no method name before parentheses
+ if (OpenParen == 0)
+ return false; // no method name before parentheses
if (Tag.back() != ')')
- return false; // some trailing rubbish after close parenthesis
+ return false; // some trailing rubbish after close parenthesis
StringRef AllParams = Tag.slice(OpenParen + 1, Tag.size() - 1);
llvm::errs() << "AllParams: " << AllParams << "\n";
if (AllParams.empty())
- return true; // verbose but valid
+ return true; // verbose but valid
// look through all the parameters
// check () and <> are balanced
@@ -6695,24 +6695,19 @@ static bool isValidAnalyzeAsMethodAttr(Decl *D,
StringRef Tag) {
// TODO currently something like blah<,> would pass validation
// we should improve this past POC stage
- for (size_t I = 0, End = AllParams.size(); I != End; ++I )
- {
+ for (size_t I = 0, End = AllParams.size(); I != End; ++I) {
char C = AllParams[I];
- if(C == '('|| C == '<'){
+ if (C == '(' || C == '<') {
BalanceCheckStack.push_back(C);
- }
- else if(C == ')')
- {
- if(BalanceCheckStack.empty() || BalanceCheckStack.back() != '(')
+ } else if (C == ')') {
+ if (BalanceCheckStack.empty() || BalanceCheckStack.back() != '(')
return false;
BalanceCheckStack.pop_back();
- }
- else if(C == '>'){
- if(BalanceCheckStack.empty() || BalanceCheckStack.back() != '<')
+ } else if (C == '>') {
+ if (BalanceCheckStack.empty() || BalanceCheckStack.back() != '<')
return false;
BalanceCheckStack.pop_back();
- }
- else if(C == ',' && BalanceCheckStack.empty()){
+ } else if (C == ',' && BalanceCheckStack.empty()) {
// TODO more detailed param checks
}
}
diff --git a/hicketts/hicketts_optional.h b/hicketts/hicketts_optional.h
index 9920039fe..62a4e98bd 100644
--- a/hicketts/hicketts_optional.h
+++ b/hicketts/hicketts_optional.h
@@ -48,10 +48,16 @@ public:
HickettsOptional(HickettsOptional &&) = default;
// Equivalent to std::optional::value()
- [[clang::analyze_as_method("value")]] const T &unwrap() const & { return
*storage_; }
+ [[clang::analyze_as_method("value")]] const T &unwrap() const & {
+ return *storage_;
+ }
[[clang::analyze_as_method("value")]] T &unwrap() & { return *storage_; }
- [[clang::analyze_as_method("value")]] const T &&unwrap() const && { return
static_cast<const T &&>(*storage_); }
- [[clang::analyze_as_method("value")]] T &&unwrap() && { return static_cast<T
&&>(*storage_); }
+ [[clang::analyze_as_method("value")]] const T &&unwrap() const && {
+ return static_cast<const T &&>(*storage_);
+ }
+ [[clang::analyze_as_method("value")]] T &&unwrap() && {
+ return static_cast<T &&>(*storage_);
+ }
const T &value() const & { return *storage_; }
T &value() & { return *storage_; }
@@ -59,49 +65,64 @@ public:
T &&value() && { return static_cast<T &&>(*storage_); }
// Equivalent to std::optional::operator*()
- [[clang::analyze_as_method("value")]] const T &deref() const & { return
*storage_; }
+ [[clang::analyze_as_method("value")]] const T &deref() const & {
+ return *storage_;
+ }
[[clang::analyze_as_method("value")]] T &deref() & { return *storage_; }
// Equivalent to std::optional::operator->()
- const T* operator ->() const { return storage_; }
- T* operator ->() { return storage_; }
+ const T *operator->() const { return storage_; }
+ T *operator->() { return storage_; }
const T *arrow() const { return storage_; }
T *arrow() { return storage_; }
// Equivalent to std::optional::operator bool / hasValue()
constexpr bool has_value() const noexcept { return storage_ != nullptr; }
- constexpr explicit operator bool() const noexcept { return storage_ !=
nullptr; }
- [[clang::analyze_as_method("has_value")]] constexpr bool isPresent() const
noexcept { return storage_ != nullptr; }
+ constexpr explicit operator bool() const noexcept {
+ return storage_ != nullptr;
+ }
+ [[clang::analyze_as_method("has_value")]] constexpr bool
+ isPresent() const noexcept {
+ return storage_ != nullptr;
+ }
// Equivalent to std::optional::value_or()
- template <typename U>
- constexpr T unwrapOr(U &&fallback) const & {
+ template <typename U> constexpr T unwrapOr(U &&fallback) const & {
return storage_ ? *storage_ : static_cast<T>(fallback);
}
// Equivalent to std::optional::emplace()
template <typename... Args>
[[clang::analyze_as_method("emplace(Args&&...)")]]
- T& construct(Args&&... args) { return *storage_; }
+ T &construct(Args &&...args) {
+ return *storage_;
+ }
- // Currently emits hicketts_optional.h:66:5: warning:
'clang::analyze_as_method' attribute argument not supported:
+ // Currently emits hicketts_optional.h:66:5: warning:
+ // 'clang::analyze_as_method' attribute argument not supported:
// 66 | [[clang::analyze_as_method("emplace(oops))")]]
[[clang::analyze_as_method("emplace(oops))")]]
- T& load() { return *storage_; }
+ T &load() {
+ return *storage_;
+ }
// Equivalent to std::optional::reset()
- [[clang::analyze_as_method("reset")]] void clear() noexcept { storage_ =
nullptr; }
+ [[clang::analyze_as_method("reset")]] void clear() noexcept {
+ storage_ = nullptr;
+ }
// Equivalent to std::optional::swap()
- [[clang::analyze_as_method("swap")]] void exchange(HickettsOptional &other)
noexcept {
+ [[clang::analyze_as_method("swap")]] void
+ exchange(HickettsOptional &other) noexcept {
T *tmp = storage_;
storage_ = other.storage_;
other.storage_ = tmp;
}
// Assignment
- template <typename U>
- HickettsOptional &operator=(const U &u) { return *this; }
+ template <typename U> HickettsOptional &operator=(const U &u) {
+ return *this;
+ }
};
} // namespace mylib
diff --git a/hicketts/test_hicketts_optional.cpp
b/hicketts/test_hicketts_optional.cpp
index 04a4526d9..7b81139c9 100644
--- a/hicketts/test_hicketts_optional.cpp
+++ b/hicketts/test_hicketts_optional.cpp
@@ -7,7 +7,8 @@
#include "hicketts_optional.h"
-// --- Unchecked access (should warn if the checker recognises
HickettsOptional) ---
+// --- Unchecked access (should warn if the checker recognises
HickettsOptional)
+// ---
static void uncheckedUnwrap(mylib::HickettsOptional<int> &Val) {
Val.unwrap(); // unchecked access — may be empty
@@ -49,8 +50,9 @@ static void checkedWithIsPresent(mylib::HickettsOptional<int>
&Val) {
// --- State changes ---
-// construct() is annotated "emplace(Args&&...)"; the bare "emplace" query
matches
-// it via the name-part (accept-either) branch -> engaged, so unwrap is safe.
+// construct() is annotated "emplace(Args&&...)"; the bare "emplace" query
+// matches it via the name-part (accept-either) branch -> engaged, so unwrap is
+// safe.
static void safeAfterConstruct(mylib::HickettsOptional<int> &Val) {
Val.construct(42);
Val.unwrap(); // safe — just constructed a value
@@ -63,7 +65,7 @@ static void unsafeAfterClear(mylib::HickettsOptional<int>
&Val) {
}
static void unsafeAfterExchange(mylib::HickettsOptional<int> &A,
- mylib::HickettsOptional<int> &B) {
+ mylib::HickettsOptional<int> &B) {
if (A) {
A.exchange(B);
A.unwrap(); // unsafe — a's state is now unknown
@@ -71,7 +73,8 @@ static void unsafeAfterExchange(mylib::HickettsOptional<int>
&A,
}
// Works today WITHOUT any annotation: default construction matches no
-// constructor case, so has_value is unconstrained -> access conservatively
warns.
+// constructor case, so has_value is unconstrained -> access conservatively
+// warns.
static void unsafeAfterEmptyConstr() {
mylib::HickettsOptional<int> A;
A.unwrap(); // expected: warn (empty)
@@ -79,8 +82,8 @@ static void unsafeAfterEmptyConstr() {
// nothing_t is not std::nullopt_t, so the structural nullopt matcher misses.
// The "optional(std::nullopt_t)" annotation routes this constructor to the
-// nullopt transfer (empty) via isOptionalNulloptConstructor's annotation
branch,
-// so the following unwrap is correctly flagged.
+// nullopt transfer (empty) via isOptionalNulloptConstructor's annotation
+// branch, so the following unwrap is correctly flagged.
static void unsafeAfterNullConstr() {
mylib::HickettsOptional<int> A(mylib::nothing);
A.unwrap(); // warns (empty) — routed to nullopt via the annotation
``````````
</details>
https://github.com/llvm/llvm-project/pull/196272
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
