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.yukhyme...@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.yukhyme...@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.yukhyme...@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.yukhyme...@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.yukhyme...@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.yukhyme...@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.yukhyme...@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.yukhyme...@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.yukhyme...@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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits