aboutsummaryrefslogtreecommitdiff
path: root/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
diff options
context:
space:
mode:
authormartinboehme <mboehme@google.com>2024-03-28 16:05:11 +0100
committerGitHub <noreply@github.com>2024-03-28 16:05:11 +0100
commitae280281ce9f14f413ced0e44158a6fd41a98243 (patch)
tree0abb50bca433d67563afec91c912a9dc8f2a1a06 /clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
parentd3aa92ed142409266ebcc9cbc20e5f2c2d0209c0 (diff)
downloadllvm-ae280281ce9f14f413ced0e44158a6fd41a98243.zip
llvm-ae280281ce9f14f413ced0e44158a6fd41a98243.tar.gz
llvm-ae280281ce9f14f413ced0e44158a6fd41a98243.tar.bz2
[clang][dataflow] Fix for value constructor in class derived from optional. (#86942)
The constructor `Derived(int)` in the newly added test `ClassDerivedFromOptionalValueConstructor` is not a template, and this used to cause an assertion failure in `valueOrConversionHasValue()` because `F.getTemplateSpecializationArgs()` returns null. (This is modeled after the `MaybeAlign(Align Value)` constructor, which similarly causes an assertion failure in the analysis when assigning an `Align` to a `MaybeAlign`.) To fix this, we can simply look at the type of the destination type which we're constructing or assigning to (instead of the function template argument), and this not only fixes this specific case but actually simplifies the implementation. I've added some additional tests for the case of assigning to a nested optional because we didn't have coverage for these and I wanted to make sure I didn't break anything.
Diffstat (limited to 'clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp')
-rw-r--r--clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp45
1 files changed, 23 insertions, 22 deletions
diff --git a/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp b/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
index dbf4878..cadb1ce 100644
--- a/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
+++ b/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
@@ -512,27 +512,26 @@ void constructOptionalValue(const Expr &E, Environment &Env,
/// Returns a symbolic value for the "has_value" property of an `optional<T>`
/// value that is constructed/assigned from a value of type `U` or `optional<U>`
/// where `T` is constructible from `U`.
-BoolValue &valueOrConversionHasValue(const FunctionDecl &F, const Expr &E,
+BoolValue &valueOrConversionHasValue(QualType DestType, const Expr &E,
const MatchFinder::MatchResult &MatchRes,
LatticeTransferState &State) {
- assert(F.getTemplateSpecializationArgs() != nullptr);
- assert(F.getTemplateSpecializationArgs()->size() > 0);
-
- const int TemplateParamOptionalWrappersCount =
- countOptionalWrappers(*MatchRes.Context, F.getTemplateSpecializationArgs()
- ->get(0)
- .getAsType()
- .getNonReferenceType());
+ const int DestTypeOptionalWrappersCount =
+ countOptionalWrappers(*MatchRes.Context, DestType);
const int ArgTypeOptionalWrappersCount = countOptionalWrappers(
*MatchRes.Context, E.getType().getNonReferenceType());
- // Check if this is a constructor/assignment call for `optional<T>` with
- // argument of type `U` such that `T` is constructible from `U`.
- if (TemplateParamOptionalWrappersCount == ArgTypeOptionalWrappersCount)
+ // Is this an constructor of the form `template<class U> optional(U &&)` /
+ // assignment of the form `template<class U> optional& operator=(U &&)`
+ // (where `T` is assignable / constructible from `U`)?
+ // We recognize this because the number of optionals in the optional being
+ // assigned to is different from the function argument type.
+ if (DestTypeOptionalWrappersCount != ArgTypeOptionalWrappersCount)
return State.Env.getBoolLiteralValue(true);
- // This is a constructor/assignment call for `optional<T>` with argument of
- // type `optional<U>` such that `T` is constructible from `U`.
+ // Otherwise, this must be a constructor of the form
+ // `template <class U> optional<optional<U> &&)` / assignment of the form
+ // `template <class U> optional& operator=(optional<U> &&)
+ // (where, again, `T` is assignable / constructible from `U`).
auto *Loc = State.Env.get<RecordStorageLocation>(E);
if (auto *HasValueVal = getHasValue(State.Env, Loc))
return *HasValueVal;
@@ -544,10 +543,11 @@ void transferValueOrConversionConstructor(
LatticeTransferState &State) {
assert(E->getNumArgs() > 0);
- constructOptionalValue(*E, State.Env,
- valueOrConversionHasValue(*E->getConstructor(),
- *E->getArg(0), MatchRes,
- State));
+ constructOptionalValue(
+ *E, State.Env,
+ valueOrConversionHasValue(
+ E->getConstructor()->getThisType()->getPointeeType(), *E->getArg(0),
+ MatchRes, State));
}
void transferAssignment(const CXXOperatorCallExpr *E, BoolValue &HasValueVal,
@@ -566,10 +566,11 @@ void transferValueOrConversionAssignment(
const CXXOperatorCallExpr *E, const MatchFinder::MatchResult &MatchRes,
LatticeTransferState &State) {
assert(E->getNumArgs() > 1);
- transferAssignment(E,
- valueOrConversionHasValue(*E->getDirectCallee(),
- *E->getArg(1), MatchRes, State),
- State);
+ transferAssignment(
+ E,
+ valueOrConversionHasValue(E->getArg(0)->getType().getNonReferenceType(),
+ *E->getArg(1), MatchRes, State),
+ State);
}
void transferNulloptAssignment(const CXXOperatorCallExpr *E,