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

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to