[clang] [clang][dataflow] Model assignment to derived class from base. (PR #85064)
https://github.com/martinboehme closed https://github.com/llvm/llvm-project/pull/85064 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][dataflow] Model assignment to derived class from base. (PR #85064)
https://github.com/Xazax-hun approved this pull request. Thanks! At this point all of my comments are addressed, and I am happy with the patch. https://github.com/llvm/llvm-project/pull/85064 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][dataflow] Model assignment to derived class from base. (PR #85064)
@@ -212,8 +212,23 @@ TEST(TransferTest, CopyRecordFromDerivedToBase) {
// [[p]]
}
)";
+ auto SyntheticFieldCallback = [](QualType Ty) -> llvm::StringMap {
+CXXRecordDecl *ADecl = nullptr;
+if (Ty.getAsString() == "A")
+ ADecl = Ty->getAsCXXRecordDecl();
+else if (Ty.getAsString() == "B")
+ ADecl = Ty->getAsCXXRecordDecl()
+ ->bases_begin()
+ ->getType()
+ ->getAsCXXRecordDecl();
+else
+ return {};
+QualType IntTy = getFieldNamed(ADecl, "i")->getType();
+return {{"synth_int", IntTy}};
+ };
+ // Copy derived to base class.
Xazax-hun wrote:
Ah, OK, I see! Thanks, I somehow expected the behavior to be present in the
code as well. Makes sense.
https://github.com/llvm/llvm-project/pull/85064
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][dataflow] Model assignment to derived class from base. (PR #85064)
@@ -14,18 +14,52 @@
#define DEBUG_TYPE "dataflow"
-void clang::dataflow::copyRecord(RecordStorageLocation &Src,
- RecordStorageLocation &Dst, Environment &Env)
{
+namespace clang::dataflow {
+
+static void copyField(const ValueDecl *Field, StorageLocation *SrcFieldLoc,
martinboehme wrote:
Yes, the swapped arguments are a consideration. I'll keep this in mind for a
possible future change.
https://github.com/llvm/llvm-project/pull/85064
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][dataflow] Model assignment to derived class from base. (PR #85064)
@@ -212,8 +212,23 @@ TEST(TransferTest, CopyRecordFromDerivedToBase) {
// [[p]]
}
)";
+ auto SyntheticFieldCallback = [](QualType Ty) -> llvm::StringMap {
+CXXRecordDecl *ADecl = nullptr;
+if (Ty.getAsString() == "A")
+ ADecl = Ty->getAsCXXRecordDecl();
+else if (Ty.getAsString() == "B")
+ ADecl = Ty->getAsCXXRecordDecl()
+ ->bases_begin()
+ ->getType()
+ ->getAsCXXRecordDecl();
+else
+ return {};
+QualType IntTy = getFieldNamed(ADecl, "i")->getType();
+return {{"synth_int", IntTy}};
+ };
+ // Copy derived to base class.
martinboehme wrote:
It doesn't happen in the analyzed code snippet at all. There's an explicit call
to `copyRecord()` below -- this is where the copy happens.
This test is trying to be a "unit test" for `copyRecord()`. The code snippet is
only there to establish the record type; the actual behavior under test is the
`copyRecord()` call (i.e. I'm intentionally calling this function directly
rather than trying to trigger a call to `copyRecord()` indirectly).
This is different from how we usually do tests in the framework (and in
compilers in general), but I think if we _can_ do unit tests relatively easily
(and in this case we can), it makes the tests clearer because you don't need to
reason about "how does this code trigger the behavior we want to test".
(There are also code-level tests for copy construction and assignment that
exercise `copyRecord()`; we obviously want to have these too.)
https://github.com/llvm/llvm-project/pull/85064
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][dataflow] Model assignment to derived class from base. (PR #85064)
https://github.com/martinboehme edited https://github.com/llvm/llvm-project/pull/85064 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][dataflow] Model assignment to derived class from base. (PR #85064)
@@ -35,45 +69,27 @@ void clang::dataflow::copyRecord(RecordStorageLocation &Src,
});
assert(compatibleTypes);
- for (auto [Field, DstFieldLoc] : Dst.children()) {
-StorageLocation *SrcFieldLoc = Src.getChild(*Field);
-
-assert(Field->getType()->isReferenceType() ||
- (SrcFieldLoc != nullptr && DstFieldLoc != nullptr));
-
-if (Field->getType()->isRecordType()) {
- copyRecord(cast(*SrcFieldLoc),
- cast(*DstFieldLoc), Env);
-} else if (Field->getType()->isReferenceType()) {
- Dst.setChild(*Field, SrcFieldLoc);
-} else {
- if (Value *Val = Env.getValue(*SrcFieldLoc))
-Env.setValue(*DstFieldLoc, *Val);
- else
-Env.clearValue(*DstFieldLoc);
-}
- }
-
- for (const auto &[Name, SynthFieldLoc] : Src.synthetic_fields()) {
-if (SynthFieldLoc->getType()->isRecordType()) {
- copyRecord(*cast(SynthFieldLoc),
- cast(Dst.getSyntheticField(Name)),
Env);
-} else {
- if (Value *Val = Env.getValue(*SynthFieldLoc))
-Env.setValue(Dst.getSyntheticField(Name), *Val);
- else
-Env.clearValue(Dst.getSyntheticField(Name));
-}
+ if (SrcType == DstType || (SrcDecl != nullptr && DstDecl != nullptr &&
+ SrcDecl->isDerivedFrom(DstDecl))) {
+for (auto [Field, DstFieldLoc] : Dst.children())
martinboehme wrote:
The two `lambda()` calls would be in the if and the else branch, respectively?
I.e.
```cxx
if (SrcType == DstType || (SrcDecl != nullptr && DstDecl != nullptr &&
SrcDecl->isDerivedFrom(DstDecl))) {
lambda(Dst);
// And synthetic fields
} else {
lambda(Src);
// And synthetic fields
}
```
The problem is that this doesn't work because it changes behavior. This is what
we have in the if branch right now:
```cxx
for (auto [Field, DstFieldLoc] : Dst.children())
copyField(Field, Src.getChild(*Field), DstFieldLoc, Dst, Env);
```
Changing this to `lambda(Dst)` would mean that the behavior becomes:
```
for (auto [Field, SrcFieldLoc] : Dst.children())
copyField(Field, SrcFieldLoc, Dst.getChild(*Field), Dst, Env);
```
Note that the name `SrcFieldLoc` is now essentially wrong, and the
`copyField()` becomes a no-op because it's copying a field to itself.
https://github.com/llvm/llvm-project/pull/85064
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][dataflow] Model assignment to derived class from base. (PR #85064)
https://github.com/Xazax-hun approved this pull request. https://github.com/llvm/llvm-project/pull/85064 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][dataflow] Model assignment to derived class from base. (PR #85064)
@@ -212,8 +212,23 @@ TEST(TransferTest, CopyRecordFromDerivedToBase) {
// [[p]]
}
)";
+ auto SyntheticFieldCallback = [](QualType Ty) -> llvm::StringMap {
+CXXRecordDecl *ADecl = nullptr;
+if (Ty.getAsString() == "A")
+ ADecl = Ty->getAsCXXRecordDecl();
+else if (Ty.getAsString() == "B")
+ ADecl = Ty->getAsCXXRecordDecl()
+ ->bases_begin()
+ ->getType()
+ ->getAsCXXRecordDecl();
+else
+ return {};
+QualType IntTy = getFieldNamed(ADecl, "i")->getType();
+return {{"synth_int", IntTy}};
+ };
+ // Copy derived to base class.
Xazax-hun wrote:
I am probably missing something. Where does the copy happen in the analyzed
code snippet?
https://github.com/llvm/llvm-project/pull/85064
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][dataflow] Model assignment to derived class from base. (PR #85064)
@@ -35,45 +69,27 @@ void clang::dataflow::copyRecord(RecordStorageLocation &Src,
});
assert(compatibleTypes);
- for (auto [Field, DstFieldLoc] : Dst.children()) {
-StorageLocation *SrcFieldLoc = Src.getChild(*Field);
-
-assert(Field->getType()->isReferenceType() ||
- (SrcFieldLoc != nullptr && DstFieldLoc != nullptr));
-
-if (Field->getType()->isRecordType()) {
- copyRecord(cast(*SrcFieldLoc),
- cast(*DstFieldLoc), Env);
-} else if (Field->getType()->isReferenceType()) {
- Dst.setChild(*Field, SrcFieldLoc);
-} else {
- if (Value *Val = Env.getValue(*SrcFieldLoc))
-Env.setValue(*DstFieldLoc, *Val);
- else
-Env.clearValue(*DstFieldLoc);
-}
- }
-
- for (const auto &[Name, SynthFieldLoc] : Src.synthetic_fields()) {
-if (SynthFieldLoc->getType()->isRecordType()) {
- copyRecord(*cast(SynthFieldLoc),
- cast(Dst.getSyntheticField(Name)),
Env);
-} else {
- if (Value *Val = Env.getValue(*SynthFieldLoc))
-Env.setValue(Dst.getSyntheticField(Name), *Val);
- else
-Env.clearValue(Dst.getSyntheticField(Name));
-}
+ if (SrcType == DstType || (SrcDecl != nullptr && DstDecl != nullptr &&
+ SrcDecl->isDerivedFrom(DstDecl))) {
+for (auto [Field, DstFieldLoc] : Dst.children())
Xazax-hun wrote:
I had something like this in mind:
```
auto lambda = [&](auto& Iter) {
for (auto [Field, SrcFieldLoc] : Iter.children())
copyField(Field, SrcFieldLoc, Dst.getChild(*Field), Dst, Env);
};
lambda(Src);
lambda(Dst);
```
That being said, something like this is not necessarily that much clearer, so
feel free to leave it as is.
https://github.com/llvm/llvm-project/pull/85064
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][dataflow] Model assignment to derived class from base. (PR #85064)
@@ -14,18 +14,52 @@
#define DEBUG_TYPE "dataflow"
-void clang::dataflow::copyRecord(RecordStorageLocation &Src,
- RecordStorageLocation &Dst, Environment &Env)
{
+namespace clang::dataflow {
+
+static void copyField(const ValueDecl *Field, StorageLocation *SrcFieldLoc,
Xazax-hun wrote:
> We could consider using StorageLocation constness to encode "can this storage
> location be written to"
I think there could be some benefit to something like this, in some cases it
could catch "swapped arguments" at compile time. That being said, I do not
insist. It is possible that it would not carry its weight.
https://github.com/llvm/llvm-project/pull/85064
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][dataflow] Model assignment to derived class from base. (PR #85064)
@@ -212,8 +212,23 @@ TEST(TransferTest, CopyRecordFromDerivedToBase) {
// [[p]]
}
)";
+ auto SyntheticFieldCallback = [](QualType Ty) -> llvm::StringMap {
+CXXRecordDecl *ADecl = nullptr;
+if (Ty.getAsString() == "A")
+ ADecl = Ty->getAsCXXRecordDecl();
+else if (Ty.getAsString() == "B")
+ ADecl = Ty->getAsCXXRecordDecl()
+ ->bases_begin()
+ ->getType()
+ ->getAsCXXRecordDecl();
+else
+ return {};
+QualType IntTy = getFieldNamed(ADecl, "i")->getType();
+return {{"synth_int", IntTy}};
+ };
+ // Copy derived to base class.
martinboehme wrote:
This means that the following lines test the case of copying from the derived
class to the base class, whereas another piece of the test further down tests
the case of copying from the base class to the derived class. I'm not sure how
I could word this better -- I've reworded slightly to "Test copying derived to
base class", as maybe it wasn't clear that I was describing the behavior to be
tested?
https://github.com/llvm/llvm-project/pull/85064
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][dataflow] Model assignment to derived class from base. (PR #85064)
https://github.com/martinboehme updated
https://github.com/llvm/llvm-project/pull/85064
>From 9362adfb0d1a61cb56b897f31fd9f2ead1605383 Mon Sep 17 00:00:00 2001
From: Martin Braenne
Date: Wed, 13 Mar 2024 12:07:24 +
Subject: [PATCH 1/3] [clang][dataflow] Model assignment to derived class from
base.
This is a relatively rare case, but
- It's still nice to get this right,
- We can remove the special case for this in `VisitCXXOperatorCallExpr()` (that
simply bails out), and
- With this in place, I can avoid having to add a similar special case in an
upcoming patch.
---
.../clang/Analysis/FlowSensitive/RecordOps.h | 6 +-
.../lib/Analysis/FlowSensitive/RecordOps.cpp | 94 +++
clang/lib/Analysis/FlowSensitive/Transfer.cpp | 9 --
.../Analysis/FlowSensitive/RecordOpsTest.cpp | 46 -
.../Analysis/FlowSensitive/TransferTest.cpp | 25 -
5 files changed, 126 insertions(+), 54 deletions(-)
diff --git a/clang/include/clang/Analysis/FlowSensitive/RecordOps.h
b/clang/include/clang/Analysis/FlowSensitive/RecordOps.h
index 783e53e980aa2c..8fad45fc11d81e 100644
--- a/clang/include/clang/Analysis/FlowSensitive/RecordOps.h
+++ b/clang/include/clang/Analysis/FlowSensitive/RecordOps.h
@@ -31,7 +31,11 @@ namespace dataflow {
///
/// Requirements:
///
-/// `Src` and `Dst` must have the same canonical unqualified type.
+/// Either:
+///- `Src` and `Dest` must have the same canonical unqualified type, or
+///- The type of `Src` must be derived from `Dest`, or
+///- The type of `Dest` must be derived from `Src` (in this case, any
fields
+/// that are only present in `Dest` are not overwritten).
void copyRecord(RecordStorageLocation &Src, RecordStorageLocation &Dst,
Environment &Env);
diff --git a/clang/lib/Analysis/FlowSensitive/RecordOps.cpp
b/clang/lib/Analysis/FlowSensitive/RecordOps.cpp
index da4dd6dc078515..4fc4c15a07a1ce 100644
--- a/clang/lib/Analysis/FlowSensitive/RecordOps.cpp
+++ b/clang/lib/Analysis/FlowSensitive/RecordOps.cpp
@@ -14,18 +14,52 @@
#define DEBUG_TYPE "dataflow"
-void clang::dataflow::copyRecord(RecordStorageLocation &Src,
- RecordStorageLocation &Dst, Environment &Env)
{
+namespace clang::dataflow {
+
+static void copyField(const ValueDecl *Field, StorageLocation *SrcFieldLoc,
+ StorageLocation *DstFieldLoc, RecordStorageLocation &Dst,
+ Environment &Env) {
+ assert(Field->getType()->isReferenceType() ||
+ (SrcFieldLoc != nullptr && DstFieldLoc != nullptr));
+
+ if (Field->getType()->isRecordType()) {
+copyRecord(cast(*SrcFieldLoc),
+ cast(*DstFieldLoc), Env);
+ } else if (Field->getType()->isReferenceType()) {
+Dst.setChild(*Field, SrcFieldLoc);
+ } else {
+if (Value *Val = Env.getValue(*SrcFieldLoc))
+ Env.setValue(*DstFieldLoc, *Val);
+else
+ Env.clearValue(*DstFieldLoc);
+ }
+}
+
+static void copySyntheticField(QualType FieldType, StorageLocation
&SrcFieldLoc,
+ StorageLocation &DstFieldLoc, Environment &Env)
{
+ if (FieldType->isRecordType()) {
+copyRecord(cast(SrcFieldLoc),
+ cast(DstFieldLoc), Env);
+ } else {
+if (Value *Val = Env.getValue(SrcFieldLoc))
+ Env.setValue(DstFieldLoc, *Val);
+else
+ Env.clearValue(DstFieldLoc);
+ }
+}
+
+void copyRecord(RecordStorageLocation &Src, RecordStorageLocation &Dst,
+Environment &Env) {
auto SrcType = Src.getType().getCanonicalType().getUnqualifiedType();
auto DstType = Dst.getType().getCanonicalType().getUnqualifiedType();
auto SrcDecl = SrcType->getAsCXXRecordDecl();
auto DstDecl = DstType->getAsCXXRecordDecl();
- bool compatibleTypes =
+ [[maybe_unused]] bool compatibleTypes =
SrcType == DstType ||
- (SrcDecl && DstDecl && SrcDecl->isDerivedFrom(DstDecl));
- (void)compatibleTypes;
+ (SrcDecl != nullptr && DstDecl != nullptr &&
+ (SrcDecl->isDerivedFrom(DstDecl) || DstDecl->isDerivedFrom(SrcDecl)));
LLVM_DEBUG({
if (!compatibleTypes) {
@@ -35,45 +69,27 @@ void clang::dataflow::copyRecord(RecordStorageLocation &Src,
});
assert(compatibleTypes);
- for (auto [Field, DstFieldLoc] : Dst.children()) {
-StorageLocation *SrcFieldLoc = Src.getChild(*Field);
-
-assert(Field->getType()->isReferenceType() ||
- (SrcFieldLoc != nullptr && DstFieldLoc != nullptr));
-
-if (Field->getType()->isRecordType()) {
- copyRecord(cast(*SrcFieldLoc),
- cast(*DstFieldLoc), Env);
-} else if (Field->getType()->isReferenceType()) {
- Dst.setChild(*Field, SrcFieldLoc);
-} else {
- if (Value *Val = Env.getValue(*SrcFieldLoc))
-Env.setValue(*DstFieldLoc, *Val);
- else
-Env.clearValue(*DstFieldLoc);
-}
- }
-
- for (const auto &[Name, SynthFieldLoc] : Src.synthetic_fields()) {
-if (SynthFieldLoc->getType()->isRecordType()) {
-
[clang] [clang][dataflow] Model assignment to derived class from base. (PR #85064)
@@ -35,45 +69,27 @@ void clang::dataflow::copyRecord(RecordStorageLocation &Src,
});
assert(compatibleTypes);
- for (auto [Field, DstFieldLoc] : Dst.children()) {
-StorageLocation *SrcFieldLoc = Src.getChild(*Field);
-
-assert(Field->getType()->isReferenceType() ||
- (SrcFieldLoc != nullptr && DstFieldLoc != nullptr));
-
-if (Field->getType()->isRecordType()) {
- copyRecord(cast(*SrcFieldLoc),
- cast(*DstFieldLoc), Env);
-} else if (Field->getType()->isReferenceType()) {
- Dst.setChild(*Field, SrcFieldLoc);
-} else {
- if (Value *Val = Env.getValue(*SrcFieldLoc))
-Env.setValue(*DstFieldLoc, *Val);
- else
-Env.clearValue(*DstFieldLoc);
-}
- }
-
- for (const auto &[Name, SynthFieldLoc] : Src.synthetic_fields()) {
-if (SynthFieldLoc->getType()->isRecordType()) {
- copyRecord(*cast(SynthFieldLoc),
- cast(Dst.getSyntheticField(Name)),
Env);
-} else {
- if (Value *Val = Env.getValue(*SynthFieldLoc))
-Env.setValue(Dst.getSyntheticField(Name), *Val);
- else
-Env.clearValue(Dst.getSyntheticField(Name));
-}
+ if (SrcType == DstType || (SrcDecl != nullptr && DstDecl != nullptr &&
+ SrcDecl->isDerivedFrom(DstDecl))) {
+for (auto [Field, DstFieldLoc] : Dst.children())
+ copyField(Field, Src.getChild(*Field), DstFieldLoc, Dst, Env);
+for (const auto &[Name, DstFieldLoc] : Dst.synthetic_fields())
+ copySyntheticField(DstFieldLoc->getType(), Src.getSyntheticField(Name),
+ *DstFieldLoc, Env);
+ } else {
+for (auto [Field, SrcFieldLoc] : Src.children())
+ copyField(Field, SrcFieldLoc, Dst.getChild(*Field), Dst, Env);
+for (const auto &[Name, SrcFieldLoc] : Src.synthetic_fields())
+ copySyntheticField(SrcFieldLoc->getType(), *SrcFieldLoc,
martinboehme wrote:
No, this is just a convention, but I'm not sure this is documented anywhere.
I'll send a PR documenting this.
https://github.com/llvm/llvm-project/pull/85064
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][dataflow] Model assignment to derived class from base. (PR #85064)
@@ -35,45 +69,27 @@ void clang::dataflow::copyRecord(RecordStorageLocation &Src,
});
assert(compatibleTypes);
- for (auto [Field, DstFieldLoc] : Dst.children()) {
-StorageLocation *SrcFieldLoc = Src.getChild(*Field);
-
-assert(Field->getType()->isReferenceType() ||
- (SrcFieldLoc != nullptr && DstFieldLoc != nullptr));
-
-if (Field->getType()->isRecordType()) {
- copyRecord(cast(*SrcFieldLoc),
- cast(*DstFieldLoc), Env);
-} else if (Field->getType()->isReferenceType()) {
- Dst.setChild(*Field, SrcFieldLoc);
-} else {
- if (Value *Val = Env.getValue(*SrcFieldLoc))
-Env.setValue(*DstFieldLoc, *Val);
- else
-Env.clearValue(*DstFieldLoc);
-}
- }
-
- for (const auto &[Name, SynthFieldLoc] : Src.synthetic_fields()) {
-if (SynthFieldLoc->getType()->isRecordType()) {
- copyRecord(*cast(SynthFieldLoc),
- cast(Dst.getSyntheticField(Name)),
Env);
-} else {
- if (Value *Val = Env.getValue(*SynthFieldLoc))
-Env.setValue(Dst.getSyntheticField(Name), *Val);
- else
-Env.clearValue(Dst.getSyntheticField(Name));
-}
+ if (SrcType == DstType || (SrcDecl != nullptr && DstDecl != nullptr &&
+ SrcDecl->isDerivedFrom(DstDecl))) {
+for (auto [Field, DstFieldLoc] : Dst.children())
martinboehme wrote:
I'm not sure exactly what you mean here? Do you mean whether we could factor
out the whole if block into a lambda so that we can reuse it in the else block?
That doesn't work because the two blocks aren't symmetrical: In one case, we
iterate over the children of `Src`, in the other we iterate over the children
of `Dst`, but in both cases, the direction of the copy is the same (source to
destination). I don't see how this could be factored out in a way that would
make the code clearer than it is now.
https://github.com/llvm/llvm-project/pull/85064
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][dataflow] Model assignment to derived class from base. (PR #85064)
@@ -14,18 +14,52 @@
#define DEBUG_TYPE "dataflow"
-void clang::dataflow::copyRecord(RecordStorageLocation &Src,
- RecordStorageLocation &Dst, Environment &Env)
{
+namespace clang::dataflow {
+
+static void copyField(const ValueDecl *Field, StorageLocation *SrcFieldLoc,
martinboehme wrote:
> Should we pass `Field` by reference?
Yes, done!
https://github.com/llvm/llvm-project/pull/85064
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][dataflow] Model assignment to derived class from base. (PR #85064)
@@ -14,18 +14,52 @@
#define DEBUG_TYPE "dataflow"
-void clang::dataflow::copyRecord(RecordStorageLocation &Src,
- RecordStorageLocation &Dst, Environment &Env)
{
+namespace clang::dataflow {
+
+static void copyField(const ValueDecl *Field, StorageLocation *SrcFieldLoc,
martinboehme wrote:
> Could `SrcFieldLoc` be a ptr to const? (Same below).
I think what you want to express is that the source field location is only ever
read from, while the destination field location is written to?
This isn't really how we use `StorageLocation` constness in the framework
today. (Maybe we should -- that's a separate discussion -- though I'm not
convinced about how much this would buy us.) Grepping for `const
StorageLocation` yields only a few hits in the whole framework[^1], and I think
that's for good reason: A `StorageLocation` is essentially immutable anyway;
all of its method are const, as are almost all of the methods in the derived
class `RecordStorageLocation`, with the exception of `setChild()` (which is a
bit of a special case). It's similar in this respect to a
[flyweight](https://en.wikipedia.org/wiki/Flyweight_pattern). So the
distinction between `const StorageLocation *` and `StorageLocation *` doesn't
mean much in practice, and I guess we've gravitated to the less verbose
`StorageLocation`.
We _could_ consider using `StorageLocation` constness to encode "can this
storage location be written to" (which I think is your intent?), which would
entail changing the storage location parameter of `setValue(const
StorageLocation &, Value &)` to be non-const, but that would be a pretty
far-reaching change because I think we'd need to add const in a lot of places
in the framework.
[^1]: Notably, `setValue(const StorageLocation &, Value &)` is one of them, so
passing a const storage location does not prevent us from writing to that
storage location.
https://github.com/llvm/llvm-project/pull/85064
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][dataflow] Model assignment to derived class from base. (PR #85064)
https://github.com/martinboehme updated
https://github.com/llvm/llvm-project/pull/85064
>From 9362adfb0d1a61cb56b897f31fd9f2ead1605383 Mon Sep 17 00:00:00 2001
From: Martin Braenne
Date: Wed, 13 Mar 2024 12:07:24 +
Subject: [PATCH 1/2] [clang][dataflow] Model assignment to derived class from
base.
This is a relatively rare case, but
- It's still nice to get this right,
- We can remove the special case for this in `VisitCXXOperatorCallExpr()` (that
simply bails out), and
- With this in place, I can avoid having to add a similar special case in an
upcoming patch.
---
.../clang/Analysis/FlowSensitive/RecordOps.h | 6 +-
.../lib/Analysis/FlowSensitive/RecordOps.cpp | 94 +++
clang/lib/Analysis/FlowSensitive/Transfer.cpp | 9 --
.../Analysis/FlowSensitive/RecordOpsTest.cpp | 46 -
.../Analysis/FlowSensitive/TransferTest.cpp | 25 -
5 files changed, 126 insertions(+), 54 deletions(-)
diff --git a/clang/include/clang/Analysis/FlowSensitive/RecordOps.h
b/clang/include/clang/Analysis/FlowSensitive/RecordOps.h
index 783e53e980aa2c..8fad45fc11d81e 100644
--- a/clang/include/clang/Analysis/FlowSensitive/RecordOps.h
+++ b/clang/include/clang/Analysis/FlowSensitive/RecordOps.h
@@ -31,7 +31,11 @@ namespace dataflow {
///
/// Requirements:
///
-/// `Src` and `Dst` must have the same canonical unqualified type.
+/// Either:
+///- `Src` and `Dest` must have the same canonical unqualified type, or
+///- The type of `Src` must be derived from `Dest`, or
+///- The type of `Dest` must be derived from `Src` (in this case, any
fields
+/// that are only present in `Dest` are not overwritten).
void copyRecord(RecordStorageLocation &Src, RecordStorageLocation &Dst,
Environment &Env);
diff --git a/clang/lib/Analysis/FlowSensitive/RecordOps.cpp
b/clang/lib/Analysis/FlowSensitive/RecordOps.cpp
index da4dd6dc078515..4fc4c15a07a1ce 100644
--- a/clang/lib/Analysis/FlowSensitive/RecordOps.cpp
+++ b/clang/lib/Analysis/FlowSensitive/RecordOps.cpp
@@ -14,18 +14,52 @@
#define DEBUG_TYPE "dataflow"
-void clang::dataflow::copyRecord(RecordStorageLocation &Src,
- RecordStorageLocation &Dst, Environment &Env)
{
+namespace clang::dataflow {
+
+static void copyField(const ValueDecl *Field, StorageLocation *SrcFieldLoc,
+ StorageLocation *DstFieldLoc, RecordStorageLocation &Dst,
+ Environment &Env) {
+ assert(Field->getType()->isReferenceType() ||
+ (SrcFieldLoc != nullptr && DstFieldLoc != nullptr));
+
+ if (Field->getType()->isRecordType()) {
+copyRecord(cast(*SrcFieldLoc),
+ cast(*DstFieldLoc), Env);
+ } else if (Field->getType()->isReferenceType()) {
+Dst.setChild(*Field, SrcFieldLoc);
+ } else {
+if (Value *Val = Env.getValue(*SrcFieldLoc))
+ Env.setValue(*DstFieldLoc, *Val);
+else
+ Env.clearValue(*DstFieldLoc);
+ }
+}
+
+static void copySyntheticField(QualType FieldType, StorageLocation
&SrcFieldLoc,
+ StorageLocation &DstFieldLoc, Environment &Env)
{
+ if (FieldType->isRecordType()) {
+copyRecord(cast(SrcFieldLoc),
+ cast(DstFieldLoc), Env);
+ } else {
+if (Value *Val = Env.getValue(SrcFieldLoc))
+ Env.setValue(DstFieldLoc, *Val);
+else
+ Env.clearValue(DstFieldLoc);
+ }
+}
+
+void copyRecord(RecordStorageLocation &Src, RecordStorageLocation &Dst,
+Environment &Env) {
auto SrcType = Src.getType().getCanonicalType().getUnqualifiedType();
auto DstType = Dst.getType().getCanonicalType().getUnqualifiedType();
auto SrcDecl = SrcType->getAsCXXRecordDecl();
auto DstDecl = DstType->getAsCXXRecordDecl();
- bool compatibleTypes =
+ [[maybe_unused]] bool compatibleTypes =
SrcType == DstType ||
- (SrcDecl && DstDecl && SrcDecl->isDerivedFrom(DstDecl));
- (void)compatibleTypes;
+ (SrcDecl != nullptr && DstDecl != nullptr &&
+ (SrcDecl->isDerivedFrom(DstDecl) || DstDecl->isDerivedFrom(SrcDecl)));
LLVM_DEBUG({
if (!compatibleTypes) {
@@ -35,45 +69,27 @@ void clang::dataflow::copyRecord(RecordStorageLocation &Src,
});
assert(compatibleTypes);
- for (auto [Field, DstFieldLoc] : Dst.children()) {
-StorageLocation *SrcFieldLoc = Src.getChild(*Field);
-
-assert(Field->getType()->isReferenceType() ||
- (SrcFieldLoc != nullptr && DstFieldLoc != nullptr));
-
-if (Field->getType()->isRecordType()) {
- copyRecord(cast(*SrcFieldLoc),
- cast(*DstFieldLoc), Env);
-} else if (Field->getType()->isReferenceType()) {
- Dst.setChild(*Field, SrcFieldLoc);
-} else {
- if (Value *Val = Env.getValue(*SrcFieldLoc))
-Env.setValue(*DstFieldLoc, *Val);
- else
-Env.clearValue(*DstFieldLoc);
-}
- }
-
- for (const auto &[Name, SynthFieldLoc] : Src.synthetic_fields()) {
-if (SynthFieldLoc->getType()->isRecordType()) {
-
[clang] [clang][dataflow] Model assignment to derived class from base. (PR #85064)
https://github.com/ymand approved this pull request. modulo Gabor's comments. https://github.com/llvm/llvm-project/pull/85064 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][dataflow] Model assignment to derived class from base. (PR #85064)
@@ -14,18 +14,52 @@
#define DEBUG_TYPE "dataflow"
-void clang::dataflow::copyRecord(RecordStorageLocation &Src,
- RecordStorageLocation &Dst, Environment &Env)
{
+namespace clang::dataflow {
+
+static void copyField(const ValueDecl *Field, StorageLocation *SrcFieldLoc,
Xazax-hun wrote:
Should we pass `Field` by reference?
https://github.com/llvm/llvm-project/pull/85064
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][dataflow] Model assignment to derived class from base. (PR #85064)
@@ -35,45 +69,27 @@ void clang::dataflow::copyRecord(RecordStorageLocation &Src,
});
assert(compatibleTypes);
- for (auto [Field, DstFieldLoc] : Dst.children()) {
-StorageLocation *SrcFieldLoc = Src.getChild(*Field);
-
-assert(Field->getType()->isReferenceType() ||
- (SrcFieldLoc != nullptr && DstFieldLoc != nullptr));
-
-if (Field->getType()->isRecordType()) {
- copyRecord(cast(*SrcFieldLoc),
- cast(*DstFieldLoc), Env);
-} else if (Field->getType()->isReferenceType()) {
- Dst.setChild(*Field, SrcFieldLoc);
-} else {
- if (Value *Val = Env.getValue(*SrcFieldLoc))
-Env.setValue(*DstFieldLoc, *Val);
- else
-Env.clearValue(*DstFieldLoc);
-}
- }
-
- for (const auto &[Name, SynthFieldLoc] : Src.synthetic_fields()) {
-if (SynthFieldLoc->getType()->isRecordType()) {
- copyRecord(*cast(SynthFieldLoc),
- cast(Dst.getSyntheticField(Name)),
Env);
-} else {
- if (Value *Val = Env.getValue(*SynthFieldLoc))
-Env.setValue(Dst.getSyntheticField(Name), *Val);
- else
-Env.clearValue(Dst.getSyntheticField(Name));
-}
+ if (SrcType == DstType || (SrcDecl != nullptr && DstDecl != nullptr &&
+ SrcDecl->isDerivedFrom(DstDecl))) {
+for (auto [Field, DstFieldLoc] : Dst.children())
+ copyField(Field, Src.getChild(*Field), DstFieldLoc, Dst, Env);
+for (const auto &[Name, DstFieldLoc] : Dst.synthetic_fields())
+ copySyntheticField(DstFieldLoc->getType(), Src.getSyntheticField(Name),
+ *DstFieldLoc, Env);
+ } else {
+for (auto [Field, SrcFieldLoc] : Src.children())
+ copyField(Field, SrcFieldLoc, Dst.getChild(*Field), Dst, Env);
+for (const auto &[Name, SrcFieldLoc] : Src.synthetic_fields())
+ copySyntheticField(SrcFieldLoc->getType(), *SrcFieldLoc,
Xazax-hun wrote:
Do we actually ensure that synthetic fields have the expected behavior (e.g.,
they are inherited)?
https://github.com/llvm/llvm-project/pull/85064
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][dataflow] Model assignment to derived class from base. (PR #85064)
@@ -35,45 +69,27 @@ void clang::dataflow::copyRecord(RecordStorageLocation &Src,
});
assert(compatibleTypes);
- for (auto [Field, DstFieldLoc] : Dst.children()) {
-StorageLocation *SrcFieldLoc = Src.getChild(*Field);
-
-assert(Field->getType()->isReferenceType() ||
- (SrcFieldLoc != nullptr && DstFieldLoc != nullptr));
-
-if (Field->getType()->isRecordType()) {
- copyRecord(cast(*SrcFieldLoc),
- cast(*DstFieldLoc), Env);
-} else if (Field->getType()->isReferenceType()) {
- Dst.setChild(*Field, SrcFieldLoc);
-} else {
- if (Value *Val = Env.getValue(*SrcFieldLoc))
-Env.setValue(*DstFieldLoc, *Val);
- else
-Env.clearValue(*DstFieldLoc);
-}
- }
-
- for (const auto &[Name, SynthFieldLoc] : Src.synthetic_fields()) {
-if (SynthFieldLoc->getType()->isRecordType()) {
- copyRecord(*cast(SynthFieldLoc),
- cast(Dst.getSyntheticField(Name)),
Env);
-} else {
- if (Value *Val = Env.getValue(*SynthFieldLoc))
-Env.setValue(Dst.getSyntheticField(Name), *Val);
- else
-Env.clearValue(Dst.getSyntheticField(Name));
-}
+ if (SrcType == DstType || (SrcDecl != nullptr && DstDecl != nullptr &&
+ SrcDecl->isDerivedFrom(DstDecl))) {
+for (auto [Field, DstFieldLoc] : Dst.children())
Xazax-hun wrote:
Alternatively, maybe this could be a lambda called twice (once with Dst, once
with Src). That being said, if it needs to capture too many things, I am fine
with it as is.
https://github.com/llvm/llvm-project/pull/85064
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][dataflow] Model assignment to derived class from base. (PR #85064)
@@ -14,18 +14,52 @@
#define DEBUG_TYPE "dataflow"
-void clang::dataflow::copyRecord(RecordStorageLocation &Src,
- RecordStorageLocation &Dst, Environment &Env)
{
+namespace clang::dataflow {
+
+static void copyField(const ValueDecl *Field, StorageLocation *SrcFieldLoc,
Xazax-hun wrote:
Could `SrcFieldLoc` be a ptr to const? (Same below).
https://github.com/llvm/llvm-project/pull/85064
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][dataflow] Model assignment to derived class from base. (PR #85064)
@@ -212,8 +212,23 @@ TEST(TransferTest, CopyRecordFromDerivedToBase) {
// [[p]]
}
)";
+ auto SyntheticFieldCallback = [](QualType Ty) -> llvm::StringMap {
+CXXRecordDecl *ADecl = nullptr;
+if (Ty.getAsString() == "A")
+ ADecl = Ty->getAsCXXRecordDecl();
+else if (Ty.getAsString() == "B")
+ ADecl = Ty->getAsCXXRecordDecl()
+ ->bases_begin()
+ ->getType()
+ ->getAsCXXRecordDecl();
+else
+ return {};
+QualType IntTy = getFieldNamed(ADecl, "i")->getType();
+return {{"synth_int", IntTy}};
+ };
+ // Copy derived to base class.
Xazax-hun wrote:
What does this comment refer to?
https://github.com/llvm/llvm-project/pull/85064
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][dataflow] Model assignment to derived class from base. (PR #85064)
llvmbot wrote:
@llvm/pr-subscribers-clang
Author: None (martinboehme)
Changes
This is a relatively rare case, but
- It's still nice to get this right,
- We can remove the special case for this in `VisitCXXOperatorCallExpr()` (that
simply bails out), and
- With this in place, I can avoid having to add a similar special case in an
upcoming patch.
---
Full diff: https://github.com/llvm/llvm-project/pull/85064.diff
5 Files Affected:
- (modified) clang/include/clang/Analysis/FlowSensitive/RecordOps.h (+5-1)
- (modified) clang/lib/Analysis/FlowSensitive/RecordOps.cpp (+56-38)
- (modified) clang/lib/Analysis/FlowSensitive/Transfer.cpp (-9)
- (modified) clang/unittests/Analysis/FlowSensitive/RecordOpsTest.cpp (+44-2)
- (modified) clang/unittests/Analysis/FlowSensitive/TransferTest.cpp (+21-4)
``diff
diff --git a/clang/include/clang/Analysis/FlowSensitive/RecordOps.h
b/clang/include/clang/Analysis/FlowSensitive/RecordOps.h
index 783e53e980aa2c..8fad45fc11d81e 100644
--- a/clang/include/clang/Analysis/FlowSensitive/RecordOps.h
+++ b/clang/include/clang/Analysis/FlowSensitive/RecordOps.h
@@ -31,7 +31,11 @@ namespace dataflow {
///
/// Requirements:
///
-/// `Src` and `Dst` must have the same canonical unqualified type.
+/// Either:
+///- `Src` and `Dest` must have the same canonical unqualified type, or
+///- The type of `Src` must be derived from `Dest`, or
+///- The type of `Dest` must be derived from `Src` (in this case, any
fields
+/// that are only present in `Dest` are not overwritten).
void copyRecord(RecordStorageLocation &Src, RecordStorageLocation &Dst,
Environment &Env);
diff --git a/clang/lib/Analysis/FlowSensitive/RecordOps.cpp
b/clang/lib/Analysis/FlowSensitive/RecordOps.cpp
index da4dd6dc078515..4fc4c15a07a1ce 100644
--- a/clang/lib/Analysis/FlowSensitive/RecordOps.cpp
+++ b/clang/lib/Analysis/FlowSensitive/RecordOps.cpp
@@ -14,18 +14,52 @@
#define DEBUG_TYPE "dataflow"
-void clang::dataflow::copyRecord(RecordStorageLocation &Src,
- RecordStorageLocation &Dst, Environment &Env)
{
+namespace clang::dataflow {
+
+static void copyField(const ValueDecl *Field, StorageLocation *SrcFieldLoc,
+ StorageLocation *DstFieldLoc, RecordStorageLocation &Dst,
+ Environment &Env) {
+ assert(Field->getType()->isReferenceType() ||
+ (SrcFieldLoc != nullptr && DstFieldLoc != nullptr));
+
+ if (Field->getType()->isRecordType()) {
+copyRecord(cast(*SrcFieldLoc),
+ cast(*DstFieldLoc), Env);
+ } else if (Field->getType()->isReferenceType()) {
+Dst.setChild(*Field, SrcFieldLoc);
+ } else {
+if (Value *Val = Env.getValue(*SrcFieldLoc))
+ Env.setValue(*DstFieldLoc, *Val);
+else
+ Env.clearValue(*DstFieldLoc);
+ }
+}
+
+static void copySyntheticField(QualType FieldType, StorageLocation
&SrcFieldLoc,
+ StorageLocation &DstFieldLoc, Environment &Env)
{
+ if (FieldType->isRecordType()) {
+copyRecord(cast(SrcFieldLoc),
+ cast(DstFieldLoc), Env);
+ } else {
+if (Value *Val = Env.getValue(SrcFieldLoc))
+ Env.setValue(DstFieldLoc, *Val);
+else
+ Env.clearValue(DstFieldLoc);
+ }
+}
+
+void copyRecord(RecordStorageLocation &Src, RecordStorageLocation &Dst,
+Environment &Env) {
auto SrcType = Src.getType().getCanonicalType().getUnqualifiedType();
auto DstType = Dst.getType().getCanonicalType().getUnqualifiedType();
auto SrcDecl = SrcType->getAsCXXRecordDecl();
auto DstDecl = DstType->getAsCXXRecordDecl();
- bool compatibleTypes =
+ [[maybe_unused]] bool compatibleTypes =
SrcType == DstType ||
- (SrcDecl && DstDecl && SrcDecl->isDerivedFrom(DstDecl));
- (void)compatibleTypes;
+ (SrcDecl != nullptr && DstDecl != nullptr &&
+ (SrcDecl->isDerivedFrom(DstDecl) || DstDecl->isDerivedFrom(SrcDecl)));
LLVM_DEBUG({
if (!compatibleTypes) {
@@ -35,45 +69,27 @@ void clang::dataflow::copyRecord(RecordStorageLocation &Src,
});
assert(compatibleTypes);
- for (auto [Field, DstFieldLoc] : Dst.children()) {
-StorageLocation *SrcFieldLoc = Src.getChild(*Field);
-
-assert(Field->getType()->isReferenceType() ||
- (SrcFieldLoc != nullptr && DstFieldLoc != nullptr));
-
-if (Field->getType()->isRecordType()) {
- copyRecord(cast(*SrcFieldLoc),
- cast(*DstFieldLoc), Env);
-} else if (Field->getType()->isReferenceType()) {
- Dst.setChild(*Field, SrcFieldLoc);
-} else {
- if (Value *Val = Env.getValue(*SrcFieldLoc))
-Env.setValue(*DstFieldLoc, *Val);
- else
-Env.clearValue(*DstFieldLoc);
-}
- }
-
- for (const auto &[Name, SynthFieldLoc] : Src.synthetic_fields()) {
-if (SynthFieldLoc->getType()->isRecordType()) {
- copyRecord(*cast(SynthFieldLoc),
- cast(Dst.getSyntheticField(Name)),
Env);
-} else {
[clang] [clang][dataflow] Model assignment to derived class from base. (PR #85064)
https://github.com/martinboehme created
https://github.com/llvm/llvm-project/pull/85064
This is a relatively rare case, but
- It's still nice to get this right,
- We can remove the special case for this in `VisitCXXOperatorCallExpr()` (that
simply bails out), and
- With this in place, I can avoid having to add a similar special case in an
upcoming patch.
>From 9362adfb0d1a61cb56b897f31fd9f2ead1605383 Mon Sep 17 00:00:00 2001
From: Martin Braenne
Date: Wed, 13 Mar 2024 12:07:24 +
Subject: [PATCH] [clang][dataflow] Model assignment to derived class from
base.
This is a relatively rare case, but
- It's still nice to get this right,
- We can remove the special case for this in `VisitCXXOperatorCallExpr()` (that
simply bails out), and
- With this in place, I can avoid having to add a similar special case in an
upcoming patch.
---
.../clang/Analysis/FlowSensitive/RecordOps.h | 6 +-
.../lib/Analysis/FlowSensitive/RecordOps.cpp | 94 +++
clang/lib/Analysis/FlowSensitive/Transfer.cpp | 9 --
.../Analysis/FlowSensitive/RecordOpsTest.cpp | 46 -
.../Analysis/FlowSensitive/TransferTest.cpp | 25 -
5 files changed, 126 insertions(+), 54 deletions(-)
diff --git a/clang/include/clang/Analysis/FlowSensitive/RecordOps.h
b/clang/include/clang/Analysis/FlowSensitive/RecordOps.h
index 783e53e980aa2c..8fad45fc11d81e 100644
--- a/clang/include/clang/Analysis/FlowSensitive/RecordOps.h
+++ b/clang/include/clang/Analysis/FlowSensitive/RecordOps.h
@@ -31,7 +31,11 @@ namespace dataflow {
///
/// Requirements:
///
-/// `Src` and `Dst` must have the same canonical unqualified type.
+/// Either:
+///- `Src` and `Dest` must have the same canonical unqualified type, or
+///- The type of `Src` must be derived from `Dest`, or
+///- The type of `Dest` must be derived from `Src` (in this case, any
fields
+/// that are only present in `Dest` are not overwritten).
void copyRecord(RecordStorageLocation &Src, RecordStorageLocation &Dst,
Environment &Env);
diff --git a/clang/lib/Analysis/FlowSensitive/RecordOps.cpp
b/clang/lib/Analysis/FlowSensitive/RecordOps.cpp
index da4dd6dc078515..4fc4c15a07a1ce 100644
--- a/clang/lib/Analysis/FlowSensitive/RecordOps.cpp
+++ b/clang/lib/Analysis/FlowSensitive/RecordOps.cpp
@@ -14,18 +14,52 @@
#define DEBUG_TYPE "dataflow"
-void clang::dataflow::copyRecord(RecordStorageLocation &Src,
- RecordStorageLocation &Dst, Environment &Env)
{
+namespace clang::dataflow {
+
+static void copyField(const ValueDecl *Field, StorageLocation *SrcFieldLoc,
+ StorageLocation *DstFieldLoc, RecordStorageLocation &Dst,
+ Environment &Env) {
+ assert(Field->getType()->isReferenceType() ||
+ (SrcFieldLoc != nullptr && DstFieldLoc != nullptr));
+
+ if (Field->getType()->isRecordType()) {
+copyRecord(cast(*SrcFieldLoc),
+ cast(*DstFieldLoc), Env);
+ } else if (Field->getType()->isReferenceType()) {
+Dst.setChild(*Field, SrcFieldLoc);
+ } else {
+if (Value *Val = Env.getValue(*SrcFieldLoc))
+ Env.setValue(*DstFieldLoc, *Val);
+else
+ Env.clearValue(*DstFieldLoc);
+ }
+}
+
+static void copySyntheticField(QualType FieldType, StorageLocation
&SrcFieldLoc,
+ StorageLocation &DstFieldLoc, Environment &Env)
{
+ if (FieldType->isRecordType()) {
+copyRecord(cast(SrcFieldLoc),
+ cast(DstFieldLoc), Env);
+ } else {
+if (Value *Val = Env.getValue(SrcFieldLoc))
+ Env.setValue(DstFieldLoc, *Val);
+else
+ Env.clearValue(DstFieldLoc);
+ }
+}
+
+void copyRecord(RecordStorageLocation &Src, RecordStorageLocation &Dst,
+Environment &Env) {
auto SrcType = Src.getType().getCanonicalType().getUnqualifiedType();
auto DstType = Dst.getType().getCanonicalType().getUnqualifiedType();
auto SrcDecl = SrcType->getAsCXXRecordDecl();
auto DstDecl = DstType->getAsCXXRecordDecl();
- bool compatibleTypes =
+ [[maybe_unused]] bool compatibleTypes =
SrcType == DstType ||
- (SrcDecl && DstDecl && SrcDecl->isDerivedFrom(DstDecl));
- (void)compatibleTypes;
+ (SrcDecl != nullptr && DstDecl != nullptr &&
+ (SrcDecl->isDerivedFrom(DstDecl) || DstDecl->isDerivedFrom(SrcDecl)));
LLVM_DEBUG({
if (!compatibleTypes) {
@@ -35,45 +69,27 @@ void clang::dataflow::copyRecord(RecordStorageLocation &Src,
});
assert(compatibleTypes);
- for (auto [Field, DstFieldLoc] : Dst.children()) {
-StorageLocation *SrcFieldLoc = Src.getChild(*Field);
-
-assert(Field->getType()->isReferenceType() ||
- (SrcFieldLoc != nullptr && DstFieldLoc != nullptr));
-
-if (Field->getType()->isRecordType()) {
- copyRecord(cast(*SrcFieldLoc),
- cast(*DstFieldLoc), Env);
-} else if (Field->getType()->isReferenceType()) {
- Dst.setChild(*Field, SrcFieldLoc);
-} else {
- if (Value *
