[PATCH] D122273: [clang][dataflow] Fix handling of base-class fields

[Stanislav Gatev via Phabricator via cfe-commits]

sgatev accepted this revision.
sgatev added inline comments.
This revision is now accepted and ready to land.


================
Comment at: clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp:168
+static void
+getFieldsFromClassHierarchy(QualType Type, bool IgnorePrivateFields,
+                            llvm::DenseSet<const FieldDecl *> &Fields) {
----------------
Let's add to the documentation of `AggregateStorageLocation` and `StructValue` that they implement a flat struct layout. I don't see an immediate reason to revisit this, but let's be explicit about it.


================
Comment at: clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp:182
+  if (auto *CXXRecord = Type->getAsCXXRecordDecl()) {
+    for (const clang::CXXBaseSpecifier &Base : CXXRecord->bases()) {
+      // Ignore private fields (including default access in C++ classes) in
----------------
The `clang` namespace is unnecessary.


================
Comment at: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp:1014
+  std::string Code = R"(
+    struct A {
+      int Bar;
----------------
Add a similar test with `class` instead of `struct`?


================
Comment at: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp:1015
+    struct A {
+      int Bar;
+    };
----------------
Let's also add private and protected members in `A` and a private member in `B`.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D122273/new/

https://reviews.llvm.org/D122273