sgatev accepted this revision.
sgatev added inline comments.
================
Comment at: clang/include/clang/Analysis/FlowSensitive/StorageLocation.h:59-60
/// can be traced independently by abstract interpretation. For example: a
-/// struct with public members.
+/// struct with public members. Note that the corresponding `StructValue` has a
+/// flat layout that does not contain the child locations stored here.
class AggregateStorageLocation final : public StorageLocation {
----------------
I find this a bit confusing. `StructValue` does not contain storage locations
in general. I think we should make it clear that the layout of
`AggregateStorageLocation` is flat, i.e. if it's used for a `struct` or `class`
type it will contain child storage locations for all accessible members of base
`struct` and `class` types.
================
Comment at: clang/include/clang/Analysis/FlowSensitive/Value.h:193-194
+/// Models a value of `struct` or `class` type. Implements a flat struct
layout,
+/// where the child values are directly reachable from the struct value (rather
+/// than indirectly, through `StorageLocation`s).
class StructValue final : public Value {
----------------
I'm not sure what's meant here by indirect reachability, but I suggest
documenting that a `StructValue` that models a `struct` or `class` type
contains child values for all accessible members of its base `struct` and
`class` types.
================
Comment at: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp:1097-1100
+ const auto *FooLoc = cast<AggregateStorageLocation>(
+ Env.getStorageLocation(*FooDecl, SkipPast::None));
+ const auto &FooVal = *cast<StructValue>(Env.getValue(*FooLoc));
+ EXPECT_THAT(FooVal.getChild(*APublicDecl), NotNull());
----------------
================
Comment at: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp:1102-1111
+ // Base-class fields.
+ EXPECT_THAT(FooVal.getChild(*ADefaultDecl), IsNull());
+ EXPECT_THAT(FooVal.getChild(*APrivateDecl), IsNull());
+ EXPECT_THAT(FooVal.getChild(*AProtectedDecl), NotNull());
+ EXPECT_THAT(FooVal.getChild(*APublicDecl), NotNull());
+
+ // Derived-class fields.
----------------
Let's also check `FooLoc`'s child storage locations. Same for the test below.
================
Comment at: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp:1156-1159
+ const auto *FooLoc = cast<AggregateStorageLocation>(
+ Env.getStorageLocation(*FooDecl, SkipPast::None));
+ const auto *FooVal = cast<StructValue>(Env.getValue(*FooLoc));
+ EXPECT_THAT(FooVal->getChild(*BarDecl), NotNull());
----------------
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D122273/new/
https://reviews.llvm.org/D122273
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
