Skip to content

Commit 5761260

Browse files
committed
Revert the init part so it is more of a no-op.
More whitespace/test cleanup
1 parent f41c0a3 commit 5761260

File tree

2 files changed

+50
-49
lines changed

2 files changed

+50
-49
lines changed

clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -583,9 +583,6 @@ bool handleConstMemberCall(const CallExpr *CE,
583583
return true;
584584
}
585585

586-
bool IsBooleanOrPointer =
587-
CE->getType()->isBooleanType() || CE->getType()->isPointerType();
588-
589586
// Cache if the const method returns a reference
590587
if (CE->isGLValue()) {
591588
const FunctionDecl *DirectCallee = CE->getDirectCallee();
@@ -595,14 +592,14 @@ bool handleConstMemberCall(const CallExpr *CE,
595592
StorageLocation &Loc =
596593
State.Lattice.getOrCreateConstMethodReturnStorageLocation(
597594
*RecordLoc, DirectCallee, State.Env, [&](StorageLocation &Loc) {
598-
if (IsBooleanOrPointer) {
599-
State.Env.setValue(Loc, *State.Env.createValue(CE->getType()));
600-
}
595+
// no-op
596+
// NOTE: if we want to support const ref to pointers or bools
597+
// we should initialize their values here.
601598
});
602599

603600
State.Env.setStorageLocation(*CE, Loc);
604601
return true;
605-
} else if (IsBooleanOrPointer) {
602+
} else if (CE->getType()->isBooleanType() || CE->getType()->isPointerType()) {
606603
// Cache if the const method returns a boolean or pointer type.
607604
Value *Val = State.Lattice.getOrCreateConstMethodReturnValue(*RecordLoc, CE,
608605
State.Env);

clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp

Lines changed: 46 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -3771,30 +3771,6 @@ TEST_P(UncheckedOptionalAccessTest, ConstPointerAccessorWithModInBetween) {
37713771
/*IgnoreSmartPointerDereference=*/false);
37723772
}
37733773

3774-
TEST_P(UncheckedOptionalAccessTest, ConstPointerRefAccessor) {
3775-
ExpectDiagnosticsFor(R"cc(
3776-
#include "unchecked_optional_access_test.h"
3777-
3778-
struct A {
3779-
$ns::$optional<int> x;
3780-
};
3781-
3782-
struct PtrWrapper {
3783-
A*& getPtrRef() const;
3784-
void reset(A*);
3785-
};
3786-
3787-
void target(PtrWrapper p) {
3788-
if (p.getPtrRef()->x) {
3789-
*p.getPtrRef()->x;
3790-
p.reset(nullptr);
3791-
*p.getPtrRef()->x; // [[unsafe]]
3792-
}
3793-
}
3794-
)cc",
3795-
/*IgnoreSmartPointerDereference=*/false);
3796-
}
3797-
37983774
TEST_P(UncheckedOptionalAccessTest, SmartPointerAccessorMixed) {
37993775
ExpectDiagnosticsFor(R"cc(
38003776
#include "unchecked_optional_access_test.h"
@@ -3853,7 +3829,7 @@ TEST_P(UncheckedOptionalAccessTest, ConstBoolAccessor) {
38533829
};
38543830
38553831
void target(A& a) {
3856-
std::optional<int> opt;
3832+
$ns::$optional<int> opt;
38573833
if (a.isFoo()) {
38583834
opt = 1;
38593835
}
@@ -3875,7 +3851,7 @@ TEST_P(UncheckedOptionalAccessTest, ConstBoolAccessorWithModInBetween) {
38753851
};
38763852
38773853
void target(A& a) {
3878-
std::optional<int> opt;
3854+
$ns::$optional<int> opt;
38793855
if (a.isFoo()) {
38803856
opt = 1;
38813857
}
@@ -3894,13 +3870,13 @@ TEST_P(UncheckedOptionalAccessTest,
38943870
38953871
struct A {
38963872
const $ns::$optional<int>& get() const { return x; }
3897-
3873+
38983874
$ns::$optional<int> x;
38993875
};
39003876
39013877
struct B {
39023878
const A& getA() const { return a; }
3903-
3879+
39043880
A a;
39053881
};
39063882
@@ -3920,13 +3896,13 @@ TEST_P(
39203896
39213897
struct A {
39223898
const $ns::$optional<int>& get() const { return x; }
3923-
3899+
39243900
$ns::$optional<int> x;
39253901
};
39263902
39273903
struct B {
39283904
const A& getA() const { return a; }
3929-
3905+
39303906
A a;
39313907
};
39323908
@@ -3943,13 +3919,13 @@ TEST_P(UncheckedOptionalAccessTest,
39433919
39443920
struct A {
39453921
const $ns::$optional<int>& get() const { return x; }
3946-
3922+
39473923
$ns::$optional<int> x;
39483924
};
39493925
39503926
struct B {
39513927
const A& getA() const { return a; }
3952-
3928+
39533929
A a;
39543930
};
39553931
@@ -3969,13 +3945,13 @@ TEST_P(UncheckedOptionalAccessTest,
39693945
39703946
struct A {
39713947
const $ns::$optional<int>& get() const { return x; }
3972-
3948+
39733949
$ns::$optional<int> x;
39743950
};
39753951
39763952
struct B {
39773953
const A copyA() const { return a; }
3978-
3954+
39793955
A a;
39803956
};
39813957
@@ -3994,13 +3970,13 @@ TEST_P(UncheckedOptionalAccessTest,
39943970
39953971
struct A {
39963972
const $ns::$optional<int>& get() const { return x; }
3997-
3973+
39983974
$ns::$optional<int> x;
39993975
};
40003976
40013977
struct B {
40023978
A& getA() { return a; }
4003-
3979+
40043980
A a;
40053981
};
40063982
@@ -4055,23 +4031,23 @@ TEST_P(
40554031
ConstRefAccessorToOptionalViaConstRefAccessorToHoldingObjectWithAnotherConstCallAfterCheck) {
40564032
ExpectDiagnosticsFor(R"cc(
40574033
#include "unchecked_optional_access_test.h"
4058-
4034+
40594035
struct A {
40604036
const $ns::$optional<int>& get() const { return x; }
4061-
4037+
40624038
$ns::$optional<int> x;
40634039
};
4064-
4040+
40654041
struct B {
40664042
const A& getA() const { return a; }
4067-
4043+
40684044
void callWithoutChanges() const {
40694045
// no-op
40704046
}
4071-
4047+
40724048
A a;
40734049
};
4074-
4050+
40754051
void target(B& b) {
40764052
if (b.getA().get().has_value()) {
40774053
b.callWithoutChanges(); // calling const method which cannot change A
@@ -4081,6 +4057,34 @@ TEST_P(
40814057
)cc");
40824058
}
40834059

4060+
TEST_P(UncheckedOptionalAccessTest, ConstPointerRefAccessor) {
4061+
// A crash reproducer for https://github.com/llvm/llvm-project/issues/125589
4062+
// NOTE: we currently cache const ref accessors's locations.
4063+
// If we want to support const ref to pointers or bools, we should initialize
4064+
// their values.
4065+
ExpectDiagnosticsFor(R"cc(
4066+
#include "unchecked_optional_access_test.h"
4067+
4068+
struct A {
4069+
$ns::$optional<int> x;
4070+
};
4071+
4072+
struct PtrWrapper {
4073+
A*& getPtrRef() const;
4074+
void reset(A*);
4075+
};
4076+
4077+
void target(PtrWrapper p) {
4078+
if (p.getPtrRef()->x) {
4079+
*p.getPtrRef()->x; // [[unsafe]]
4080+
p.reset(nullptr);
4081+
*p.getPtrRef()->x; // [[unsafe]]
4082+
}
4083+
}
4084+
)cc",
4085+
/*IgnoreSmartPointerDereference=*/false);
4086+
}
4087+
40844088
// FIXME: Add support for:
40854089
// - constructors (copy, move)
40864090
// - assignment operators (default, copy, move)

0 commit comments

Comments
 (0)