[clang] [clang-tools-extra] [clang-tidy] [dataflow] Cache reference accessors for `bugprone-unchecked-optional-access` (PR #128437)
Valentyn Yukhymenko via cfe-commits
cfe-commits at lists.llvm.org
Fri Feb 28 06:56:41 PST 2025
https://github.com/BaLiKfromUA updated https://github.com/llvm/llvm-project/pull/128437
>From 319ad0b803b8c6c6c5405178335bd1f2258be4b8 Mon Sep 17 00:00:00 2001
From: Valentyn Yukhymenko <valentin.yukhymenko at gmail.com>
Date: Sun, 23 Feb 2025 12:08:02 +0000
Subject: [PATCH 1/9] first implementation and basic tests
---
.../Models/UncheckedOptionalAccessModel.cpp | 20 +++++++
.../UncheckedOptionalAccessModelTest.cpp | 59 +++++++++++++++++++
2 files changed, 79 insertions(+)
diff --git a/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp b/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
index e1394e28cd49a..993967e0c3edd 100644
--- a/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
+++ b/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
@@ -580,6 +580,26 @@ void handleConstMemberCall(const CallExpr *CE,
return;
}
+ // if const method returns a reference
+ if (CE->isGLValue()) {
+ const FunctionDecl *DirectCallee = CE->getDirectCallee();
+ if (DirectCallee == nullptr)
+ return;
+
+ QualType DeclaredReturnType = DirectCallee->getReturnType();
+
+ if (DeclaredReturnType.getTypePtr()->isReferenceType()) {
+ StorageLocation &Loc =
+ State.Lattice.getOrCreateConstMethodReturnStorageLocation(
+ *RecordLoc, DirectCallee, State.Env, [&](StorageLocation &Loc) {
+ // No-op
+ });
+
+ State.Env.setStorageLocation(*CE, Loc);
+ return;
+ }
+ }
+
// Cache if the const method returns a boolean or pointer type.
// We may decide to cache other return types in the future.
if (RecordLoc != nullptr &&
diff --git a/clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp b/clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
index 19c3ff49eab27..7140040022794 100644
--- a/clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
+++ b/clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
@@ -3863,6 +3863,65 @@ TEST_P(UncheckedOptionalAccessTest, ConstBoolAccessorWithModInBetween) {
)cc");
}
+
+TEST_P(UncheckedOptionalAccessTest, ConstRefAccessorToOptionalViaConstRefAccessorToHoldingObject) {
+ ExpectDiagnosticsFor(R"cc(
+ #include "unchecked_optional_access_test.h"
+
+ class A {
+ public:
+ const $ns::$optional<int>& get() const { return x; }
+
+ private:
+ $ns::$optional<int> x;
+ };
+
+ class B {
+ public:
+ const A& getA() const { return a; }
+
+ private:
+ A a;
+ };
+
+ void target(B& b) {
+ if (b.getA().get().has_value()) {
+ b.getA().get().value();
+ }
+ }
+ )cc");
+}
+
+TEST_P(UncheckedOptionalAccessTest, ConstRefAccessorToOptionalViaConstRefAccessorToHoldingObjectWithoutValueCheck) {
+ ExpectDiagnosticsFor(R"cc(
+ #include "unchecked_optional_access_test.h"
+
+ class A {
+ public:
+ const $ns::$optional<int>& get() const { return x; }
+
+ private:
+ $ns::$optional<int> x;
+ };
+
+ class B {
+ public:
+ const A& getA() const { return a; }
+
+ private:
+ A a;
+ };
+
+ void target(B& b) {
+ b.getA().get().value(); // [[unsafe]]
+ }
+ )cc");
+}
+
+// todo: non const accessor
+// todo: different accessor in between
+// todo: const copy
+
// FIXME: Add support for:
// - constructors (copy, move)
// - assignment operators (default, copy, move)
>From d7e3105087d5347fe100f0a567c1538c1a3673c0 Mon Sep 17 00:00:00 2001
From: Valentyn Yukhymenko <valentin.yukhymenko at gmail.com>
Date: Sun, 23 Feb 2025 21:37:54 +0000
Subject: [PATCH 2/9] more tests
---
.../Models/UncheckedOptionalAccessModel.cpp | 11 +-
.../UncheckedOptionalAccessModelTest.cpp | 126 +++++++++++++++++-
2 files changed, 128 insertions(+), 9 deletions(-)
diff --git a/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp b/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
index 993967e0c3edd..a35ac09b15502 100644
--- a/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
+++ b/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
@@ -580,19 +580,18 @@ void handleConstMemberCall(const CallExpr *CE,
return;
}
- // if const method returns a reference
- if (CE->isGLValue()) {
+ // Cache if the const method returns a reference
+ if (RecordLoc != nullptr && CE->isGLValue()) {
const FunctionDecl *DirectCallee = CE->getDirectCallee();
if (DirectCallee == nullptr)
return;
- QualType DeclaredReturnType = DirectCallee->getReturnType();
-
- if (DeclaredReturnType.getTypePtr()->isReferenceType()) {
+ bool isReference = DirectCallee->getReturnType().getTypePtr()->isReferenceType();
+ if (isReference) {
StorageLocation &Loc =
State.Lattice.getOrCreateConstMethodReturnStorageLocation(
*RecordLoc, DirectCallee, State.Env, [&](StorageLocation &Loc) {
- // No-op
+ // no-op
});
State.Env.setStorageLocation(*CE, Loc);
diff --git a/clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp b/clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
index 7140040022794..4cec24829885c 100644
--- a/clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
+++ b/clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
@@ -3918,9 +3918,129 @@ TEST_P(UncheckedOptionalAccessTest, ConstRefAccessorToOptionalViaConstRefAccesso
)cc");
}
-// todo: non const accessor
-// todo: different accessor in between
-// todo: const copy
+TEST_P(UncheckedOptionalAccessTest, ConstRefToOptionalSavedAsTemporaryVariable) {
+ ExpectDiagnosticsFor(R"cc(
+ #include "unchecked_optional_access_test.h"
+
+ class A {
+ public:
+ const $ns::$optional<int>& get() const { return x; }
+
+ private:
+ $ns::$optional<int> x;
+ };
+
+ class B {
+ public:
+ const A& getA() const { return a; }
+
+ private:
+ A a;
+ };
+
+ void target(B& b) {
+ const auto& opt = b.getA().get();
+ if (opt.has_value()) {
+ opt.value();
+ }
+ }
+ )cc");
+}
+
+TEST_P(UncheckedOptionalAccessTest, ConstRefAccessorToOptionalViaConstValueAccessorToHoldingObject) {
+ ExpectDiagnosticsFor(R"cc(
+ #include "unchecked_optional_access_test.h"
+
+ class A {
+ public:
+ const $ns::$optional<int>& get() const { return x; }
+
+ private:
+ $ns::$optional<int> x;
+ };
+
+ class B {
+ public:
+ const A copyA() const { return a; }
+
+ private:
+ A a;
+ };
+
+ void target(B& b) {
+ if (b.copyA().get().has_value()) {
+ b.copyA().get().value(); // [[unsafe]]
+ }
+ }
+ )cc");
+}
+
+TEST_P(UncheckedOptionalAccessTest, ConstRefAccessorToOptionalViaNonConstRefAccessorToHoldingObject) {
+ ExpectDiagnosticsFor(R"cc(
+ #include "unchecked_optional_access_test.h"
+
+ class A {
+ public:
+ const $ns::$optional<int>& get() const { return x; }
+
+ private:
+ $ns::$optional<int> x;
+ };
+
+ class B {
+ public:
+ A& getA() { return a; }
+
+ private:
+ A a;
+ };
+
+ void target(B& b) {
+ if (b.getA().get().has_value()) {
+ b.getA().get().value(); // [[unsafe]]
+ }
+ }
+ )cc");
+}
+
+TEST_P(UncheckedOptionalAccessTest, ConstRefAccessorToOptionalViaNonConstRefAccessorToHoldingObjectWithModAfterCheck) {
+ ExpectDiagnosticsFor(R"cc(
+ #include "unchecked_optional_access_test.h"
+
+ class A {
+ public:
+ const $ns::$optional<int>& get() const { return x; }
+ private:
+ $ns::$optional<int> x;
+ };
+
+ class B {
+ public:
+ const A& getA() const { return a; }
+
+ A& getA() { return a; }
+
+ void clear() { a = A{}; };
+
+ private:
+ A a;
+ };
+
+ void target(B& b) {
+ // changing field A via non-const getter after const getter check
+ if (b.getA().get().has_value()) {
+ b.getA() = A{};
+ b.getA().get().value(); // [[unsafe]]
+ }
+
+ // calling non-const method which might change field A
+ if (b.getA().get().has_value()) {
+ b.clear();
+ b.getA().get().value(); // [[unsafe]]
+ }
+ }
+ )cc");
+}
// FIXME: Add support for:
// - constructors (copy, move)
>From 9608e954136b6cd8ee51ce5a301b828caadb314e Mon Sep 17 00:00:00 2001
From: Valentyn Yukhymenko <valentin.yukhymenko at gmail.com>
Date: Sun, 23 Feb 2025 21:42:36 +0000
Subject: [PATCH 3/9] format
---
.../Models/UncheckedOptionalAccessModel.cpp | 13 ++++++------
.../UncheckedOptionalAccessModelTest.cpp | 21 ++++++++++++-------
2 files changed, 21 insertions(+), 13 deletions(-)
diff --git a/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp b/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
index a35ac09b15502..dccf5ee7f94c2 100644
--- a/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
+++ b/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
@@ -586,14 +586,15 @@ void handleConstMemberCall(const CallExpr *CE,
if (DirectCallee == nullptr)
return;
- bool isReference = DirectCallee->getReturnType().getTypePtr()->isReferenceType();
+ bool isReference =
+ DirectCallee->getReturnType().getTypePtr()->isReferenceType();
if (isReference) {
StorageLocation &Loc =
- State.Lattice.getOrCreateConstMethodReturnStorageLocation(
- *RecordLoc, DirectCallee, State.Env, [&](StorageLocation &Loc) {
- // no-op
- });
-
+ State.Lattice.getOrCreateConstMethodReturnStorageLocation(
+ *RecordLoc, DirectCallee, State.Env, [&](StorageLocation &Loc) {
+ // no-op
+ });
+
State.Env.setStorageLocation(*CE, Loc);
return;
}
diff --git a/clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp b/clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
index 4cec24829885c..ddecab3af449d 100644
--- a/clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
+++ b/clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
@@ -3863,8 +3863,8 @@ TEST_P(UncheckedOptionalAccessTest, ConstBoolAccessorWithModInBetween) {
)cc");
}
-
-TEST_P(UncheckedOptionalAccessTest, ConstRefAccessorToOptionalViaConstRefAccessorToHoldingObject) {
+TEST_P(UncheckedOptionalAccessTest,
+ ConstRefAccessorToOptionalViaConstRefAccessorToHoldingObject) {
ExpectDiagnosticsFor(R"cc(
#include "unchecked_optional_access_test.h"
@@ -3892,7 +3892,9 @@ TEST_P(UncheckedOptionalAccessTest, ConstRefAccessorToOptionalViaConstRefAccesso
)cc");
}
-TEST_P(UncheckedOptionalAccessTest, ConstRefAccessorToOptionalViaConstRefAccessorToHoldingObjectWithoutValueCheck) {
+TEST_P(
+ UncheckedOptionalAccessTest,
+ ConstRefAccessorToOptionalViaConstRefAccessorToHoldingObjectWithoutValueCheck) {
ExpectDiagnosticsFor(R"cc(
#include "unchecked_optional_access_test.h"
@@ -3918,7 +3920,8 @@ TEST_P(UncheckedOptionalAccessTest, ConstRefAccessorToOptionalViaConstRefAccesso
)cc");
}
-TEST_P(UncheckedOptionalAccessTest, ConstRefToOptionalSavedAsTemporaryVariable) {
+TEST_P(UncheckedOptionalAccessTest,
+ ConstRefToOptionalSavedAsTemporaryVariable) {
ExpectDiagnosticsFor(R"cc(
#include "unchecked_optional_access_test.h"
@@ -3947,7 +3950,8 @@ TEST_P(UncheckedOptionalAccessTest, ConstRefToOptionalSavedAsTemporaryVariable)
)cc");
}
-TEST_P(UncheckedOptionalAccessTest, ConstRefAccessorToOptionalViaConstValueAccessorToHoldingObject) {
+TEST_P(UncheckedOptionalAccessTest,
+ ConstRefAccessorToOptionalViaConstValueAccessorToHoldingObject) {
ExpectDiagnosticsFor(R"cc(
#include "unchecked_optional_access_test.h"
@@ -3975,7 +3979,8 @@ TEST_P(UncheckedOptionalAccessTest, ConstRefAccessorToOptionalViaConstValueAcces
)cc");
}
-TEST_P(UncheckedOptionalAccessTest, ConstRefAccessorToOptionalViaNonConstRefAccessorToHoldingObject) {
+TEST_P(UncheckedOptionalAccessTest,
+ ConstRefAccessorToOptionalViaNonConstRefAccessorToHoldingObject) {
ExpectDiagnosticsFor(R"cc(
#include "unchecked_optional_access_test.h"
@@ -4003,7 +4008,9 @@ TEST_P(UncheckedOptionalAccessTest, ConstRefAccessorToOptionalViaNonConstRefAcce
)cc");
}
-TEST_P(UncheckedOptionalAccessTest, ConstRefAccessorToOptionalViaNonConstRefAccessorToHoldingObjectWithModAfterCheck) {
+TEST_P(
+ UncheckedOptionalAccessTest,
+ ConstRefAccessorToOptionalViaNonConstRefAccessorToHoldingObjectWithModAfterCheck) {
ExpectDiagnosticsFor(R"cc(
#include "unchecked_optional_access_test.h"
>From 011f8065e5c2671c933bcf4be18193e89e468ab5 Mon Sep 17 00:00:00 2001
From: Valentyn Yukhymenko <valentin.yukhymenko at gmail.com>
Date: Wed, 26 Feb 2025 22:57:53 +0000
Subject: [PATCH 4/9] remove redundant reference check and add one more test
---
.../Models/UncheckedOptionalAccessModel.cpp | 18 ++++-----
.../UncheckedOptionalAccessModelTest.cpp | 39 ++++++++++++++++++-
2 files changed, 44 insertions(+), 13 deletions(-)
diff --git a/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp b/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
index dccf5ee7f94c2..f9b10bdb62154 100644
--- a/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
+++ b/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
@@ -586,18 +586,14 @@ void handleConstMemberCall(const CallExpr *CE,
if (DirectCallee == nullptr)
return;
- bool isReference =
- DirectCallee->getReturnType().getTypePtr()->isReferenceType();
- if (isReference) {
- StorageLocation &Loc =
- State.Lattice.getOrCreateConstMethodReturnStorageLocation(
- *RecordLoc, DirectCallee, State.Env, [&](StorageLocation &Loc) {
- // no-op
- });
+ StorageLocation &Loc =
+ State.Lattice.getOrCreateConstMethodReturnStorageLocation(
+ *RecordLoc, DirectCallee, State.Env, [&](StorageLocation &Loc) {
+ // no-op
+ });
- State.Env.setStorageLocation(*CE, Loc);
- return;
- }
+ State.Env.setStorageLocation(*CE, Loc);
+ return;
}
// Cache if the const method returns a boolean or pointer type.
diff --git a/clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp b/clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
index ddecab3af449d..0aee8f0067242 100644
--- a/clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
+++ b/clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
@@ -4010,7 +4010,7 @@ TEST_P(UncheckedOptionalAccessTest,
TEST_P(
UncheckedOptionalAccessTest,
- ConstRefAccessorToOptionalViaNonConstRefAccessorToHoldingObjectWithModAfterCheck) {
+ ConstRefAccessorToOptionalViaConstRefAccessorToHoldingObjectWithModAfterCheck) {
ExpectDiagnosticsFor(R"cc(
#include "unchecked_optional_access_test.h"
@@ -4027,7 +4027,7 @@ TEST_P(
A& getA() { return a; }
- void clear() { a = A{}; };
+ void clear() { a = A{}; }
private:
A a;
@@ -4049,6 +4049,41 @@ TEST_P(
)cc");
}
+
+TEST_P(
+ UncheckedOptionalAccessTest,
+ ConstRefAccessorToOptionalViaConstRefAccessorToHoldingObjectWithAnotherConstCallAfterCheck) {
+ ExpectDiagnosticsFor(R"cc(
+ #include "unchecked_optional_access_test.h"
+
+ class A {
+ public:
+ const $ns::$optional<int>& get() const { return x; }
+ private:
+ $ns::$optional<int> x;
+ };
+
+ class B {
+ public:
+ const A& getA() const { return a; }
+
+ void callWithoutChanges() const {
+ // no-op
+ }
+
+ private:
+ A a;
+ };
+
+ void target(B& b) {
+ if (b.getA().get().has_value()) {
+ b.callWithoutChanges(); // calling const method which cannot change A
+ b.getA().get().value();
+ }
+ }
+ )cc");
+}
+
// FIXME: Add support for:
// - constructors (copy, move)
// - assignment operators (default, copy, move)
>From a46121eb8b6016779790741cf8744cfe38319aa8 Mon Sep 17 00:00:00 2001
From: Valentyn Yukhymenko <valentin.yukhymenko at gmail.com>
Date: Wed, 26 Feb 2025 22:59:33 +0000
Subject: [PATCH 5/9] format
---
.../FlowSensitive/Models/UncheckedOptionalAccessModel.cpp | 2 +-
.../FlowSensitive/UncheckedOptionalAccessModelTest.cpp | 7 +++----
2 files changed, 4 insertions(+), 5 deletions(-)
diff --git a/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp b/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
index f9b10bdb62154..9381c5c42e566 100644
--- a/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
+++ b/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
@@ -590,7 +590,7 @@ void handleConstMemberCall(const CallExpr *CE,
State.Lattice.getOrCreateConstMethodReturnStorageLocation(
*RecordLoc, DirectCallee, State.Env, [&](StorageLocation &Loc) {
// no-op
- });
+ });
State.Env.setStorageLocation(*CE, Loc);
return;
diff --git a/clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp b/clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
index 0aee8f0067242..07269efaece26 100644
--- a/clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
+++ b/clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
@@ -4049,11 +4049,10 @@ TEST_P(
)cc");
}
-
TEST_P(
- UncheckedOptionalAccessTest,
- ConstRefAccessorToOptionalViaConstRefAccessorToHoldingObjectWithAnotherConstCallAfterCheck) {
- ExpectDiagnosticsFor(R"cc(
+ UncheckedOptionalAccessTest,
+ ConstRefAccessorToOptionalViaConstRefAccessorToHoldingObjectWithAnotherConstCallAfterCheck) {
+ ExpectDiagnosticsFor(R"cc(
#include "unchecked_optional_access_test.h"
class A {
>From cf35ecf1336ab0fac850458efc7df54ee7fb036a Mon Sep 17 00:00:00 2001
From: Valentyn Yukhymenko <valentin.yukhymenko at gmail.com>
Date: Wed, 26 Feb 2025 23:47:23 +0000
Subject: [PATCH 6/9] simplify tests
---
.../UncheckedOptionalAccessModelTest.cpp | 126 +++++++-----------
1 file changed, 50 insertions(+), 76 deletions(-)
diff --git a/clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp b/clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
index 07269efaece26..5031e17188e17 100644
--- a/clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
+++ b/clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
@@ -3868,20 +3868,16 @@ TEST_P(UncheckedOptionalAccessTest,
ExpectDiagnosticsFor(R"cc(
#include "unchecked_optional_access_test.h"
- class A {
- public:
- const $ns::$optional<int>& get() const { return x; }
+ struct A {
+ const $ns::$optional<int>& get() const { return x; }
- private:
- $ns::$optional<int> x;
+ $ns::$optional<int> x;
};
- class B {
- public:
- const A& getA() const { return a; }
+ struct B {
+ const A& getA() const { return a; }
- private:
- A a;
+ A a;
};
void target(B& b) {
@@ -3898,20 +3894,16 @@ TEST_P(
ExpectDiagnosticsFor(R"cc(
#include "unchecked_optional_access_test.h"
- class A {
- public:
- const $ns::$optional<int>& get() const { return x; }
+ struct A {
+ const $ns::$optional<int>& get() const { return x; }
- private:
- $ns::$optional<int> x;
+ $ns::$optional<int> x;
};
- class B {
- public:
- const A& getA() const { return a; }
+ struct B {
+ const A& getA() const { return a; }
- private:
- A a;
+ A a;
};
void target(B& b) {
@@ -3925,20 +3917,16 @@ TEST_P(UncheckedOptionalAccessTest,
ExpectDiagnosticsFor(R"cc(
#include "unchecked_optional_access_test.h"
- class A {
- public:
- const $ns::$optional<int>& get() const { return x; }
+ struct A {
+ const $ns::$optional<int>& get() const { return x; }
- private:
- $ns::$optional<int> x;
+ $ns::$optional<int> x;
};
- class B {
- public:
- const A& getA() const { return a; }
+ struct B {
+ const A& getA() const { return a; }
- private:
- A a;
+ A a;
};
void target(B& b) {
@@ -3951,24 +3939,20 @@ TEST_P(UncheckedOptionalAccessTest,
}
TEST_P(UncheckedOptionalAccessTest,
- ConstRefAccessorToOptionalViaConstValueAccessorToHoldingObject) {
+ ConstRefAccessorToOptionalViaAccessorToHoldingObjectByValue) {
ExpectDiagnosticsFor(R"cc(
#include "unchecked_optional_access_test.h"
- class A {
- public:
- const $ns::$optional<int>& get() const { return x; }
+ struct A {
+ const $ns::$optional<int>& get() const { return x; }
- private:
- $ns::$optional<int> x;
+ $ns::$optional<int> x;
};
- class B {
- public:
- const A copyA() const { return a; }
+ struct B {
+ const A copyA() const { return a; }
- private:
- A a;
+ A a;
};
void target(B& b) {
@@ -3984,20 +3968,16 @@ TEST_P(UncheckedOptionalAccessTest,
ExpectDiagnosticsFor(R"cc(
#include "unchecked_optional_access_test.h"
- class A {
- public:
- const $ns::$optional<int>& get() const { return x; }
+ struct A {
+ const $ns::$optional<int>& get() const { return x; }
- private:
- $ns::$optional<int> x;
+ $ns::$optional<int> x;
};
- class B {
- public:
- A& getA() { return a; }
+ struct B {
+ A& getA() { return a; }
- private:
- A a;
+ A a;
};
void target(B& b) {
@@ -4014,23 +3994,20 @@ TEST_P(
ExpectDiagnosticsFor(R"cc(
#include "unchecked_optional_access_test.h"
- class A {
- public:
- const $ns::$optional<int>& get() const { return x; }
- private:
- $ns::$optional<int> x;
+ struct A {
+ const $ns::$optional<int>& get() const { return x; }
+
+ $ns::$optional<int> x;
};
- class B {
- public:
- const A& getA() const { return a; }
+ struct B {
+ const A& getA() const { return a; }
- A& getA() { return a; }
+ A& getA() { return a; }
- void clear() { a = A{}; }
+ void clear() { a = A{}; }
- private:
- A a;
+ A a;
};
void target(B& b) {
@@ -4055,23 +4032,20 @@ TEST_P(
ExpectDiagnosticsFor(R"cc(
#include "unchecked_optional_access_test.h"
- class A {
- public:
- const $ns::$optional<int>& get() const { return x; }
- private:
- $ns::$optional<int> x;
+ struct A {
+ const $ns::$optional<int>& get() const { return x; }
+
+ $ns::$optional<int> x;
};
- class B {
- public:
- const A& getA() const { return a; }
+ struct B {
+ const A& getA() const { return a; }
- void callWithoutChanges() const {
- // no-op
- }
+ void callWithoutChanges() const {
+ // no-op
+ }
- private:
- A a;
+ A a;
};
void target(B& b) {
>From ad9bbd740f4ef32014aa43c95c24d5d94ecfc49e Mon Sep 17 00:00:00 2001
From: Valentyn Yukhymenko <valentin.yukhymenko at gmail.com>
Date: Thu, 27 Feb 2025 00:05:36 +0000
Subject: [PATCH 7/9] update release notes
---
clang-tools-extra/docs/ReleaseNotes.rst | 2 ++
1 file changed, 2 insertions(+)
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 3bddeeda06e06..dfa4cb1d47150 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -100,6 +100,8 @@ Changes in existing checks
- Improved :doc:`bugprone-unsafe-functions
<clang-tidy/checks/bugprone/unsafe-functions>` check to allow specifying
additional C++ member functions to match.
+- Improved :doc:`bugprone-unchecked-optional-access
+ <clang-tidy/checks/bugprone/unchecked-optional-access>` by fixing false positive from const reference accessors to objects containing optional member.
Removed checks
^^^^^^^^^^^^^^
>From 0e103f61ee98fed990c324a6acd2b8469450972f Mon Sep 17 00:00:00 2001
From: Valentyn Yukhymenko <valentin.yukhymenko at gmail.com>
Date: Thu, 27 Feb 2025 23:34:01 +0000
Subject: [PATCH 8/9] reorder release notes
---
clang-tools-extra/docs/ReleaseNotes.rst | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 171f1fcc6a092..12db8fc781ad7 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -114,11 +114,12 @@ Changes in existing checks
and accessing ``value``. The option `IgnoreSmartPointerDereference` should
no longer be needed and will be removed.
+- Improved :doc:`bugprone-unchecked-optional-access
+ <clang-tidy/checks/bugprone/unchecked-optional-access>` by fixing false positive from const reference accessors to objects containing optional member.
+
- Improved :doc:`bugprone-unsafe-functions
<clang-tidy/checks/bugprone/unsafe-functions>` check to allow specifying
additional C++ member functions to match.
-- Improved :doc:`bugprone-unchecked-optional-access
- <clang-tidy/checks/bugprone/unchecked-optional-access>` by fixing false positive from const reference accessors to objects containing optional member.
- Improved :doc:`misc-const-correctness
<clang-tidy/checks/misc/const-correctness>` check by adding the option
>From 36cf5eb25d58214e5bf231bb2a202aeb49d02558 Mon Sep 17 00:00:00 2001
From: Valentyn Yukhymenko <valentin.yukhymenko at gmail.com>
Date: Fri, 28 Feb 2025 14:56:29 +0000
Subject: [PATCH 9/9] Update ReleaseNotes.rst
---
clang-tools-extra/docs/ReleaseNotes.rst | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 12db8fc781ad7..07a79d6bbe807 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -112,10 +112,8 @@ Changes in existing checks
<clang-tidy/checks/bugprone/unchecked-optional-access>` fixing false
positives from smart pointer accessors repeated in checking ``has_value``
and accessing ``value``. The option `IgnoreSmartPointerDereference` should
- no longer be needed and will be removed.
-
-- Improved :doc:`bugprone-unchecked-optional-access
- <clang-tidy/checks/bugprone/unchecked-optional-access>` by fixing false positive from const reference accessors to objects containing optional member.
+ no longer be needed and will be removed. Also fixing false positive from
+ const reference accessors to objects containing optional member.
- Improved :doc:`bugprone-unsafe-functions
<clang-tidy/checks/bugprone/unsafe-functions>` check to allow specifying
More information about the cfe-commits
mailing list