Addressed comments

Author: kinu

clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp

@@ -288,15 +288,12 @@ static void insertIfFunction(const Decl &D,
     Funcs.insert(FD);
 }
 
-static Expr *getRetValueFromSingleReturnStmtMethod(const CXXMemberCallExpr &C) {
-  auto *D = cast_or_null<CXXMethodDecl>(C.getMethodDecl()->getDefinition());
-  if (!D)
+static MemberExpr *getMemberForAccessor(const CXXMemberCallExpr &C) {
+  auto *Body = dyn_cast_or_null<CompoundStmt>(C.getMethodDecl()->getBody());
+  if (!Body || Body->size() != 1)
     return nullptr;
-  auto *S = cast<CompoundStmt>(D->getBody());
-  if (S->size() != 1)
-    return nullptr;
-  if (auto *RS = dyn_cast<ReturnStmt>(*S->body_begin()))
-    return RS->getRetValue()->IgnoreParenImpCasts();
+  if (auto *RS = dyn_cast<ReturnStmt>(*Body->body_begin()))
+    return dyn_cast<MemberExpr>(RS->getRetValue()->IgnoreParenImpCasts());
   return nullptr;
 }
 
@@ -339,9 +336,9 @@ getFieldsGlobalsAndFuncs(const Stmt &S, FieldSet &Fields,
   } else if (const auto *C = dyn_cast<CXXMemberCallExpr>(&S)) {
     // If this is a method that returns a member variable but does nothing else,
     // model the field of the return value.
-    if (MemberExpr *E = dyn_cast_or_null<MemberExpr>(
-        getRetValueFromSingleReturnStmtMethod(*C)))
-      getFieldsGlobalsAndFuncs(*E, Fields, Vars, Funcs);
+    if (MemberExpr *E = getMemberForAccessor(*C))
+      if (const auto *FD = dyn_cast<FieldDecl>(E->getMemberDecl()))
+        Fields.insert(FD);
   } else if (auto *E = dyn_cast<MemberExpr>(&S)) {
     // FIXME: should we be using `E->getFoundDecl()`?
     const ValueDecl *VD = E->getMemberDecl();

clang/unittests/Analysis/FlowSensitive/TransferTest.cpp

@@ -1449,27 +1449,30 @@ TEST(TransferTest, BaseClassInitializer) {
 TEST(TransferTest, StructModeledFieldsWithAccessor) {
   std::string Code = R"(
     class S {
-      int *P;
-      int *Q;
-      int X;
-      int Y;
-      int Z;
+      int *Ptr;
+      int *PtrNonConst;
+      int Int;
+      int IntWithInc;
+      int IntNotAccessed;
+      int IntRef;
     public:
-      int *getPtr() const { return P; }
-      int *getPtrNonConst() { return Q; }
-      int getInt(int i) const { return X; }
-      int getWithOtherWork(int i) { Y += i; return Y; }
-      int getIntNotCalled() const { return Z; }
+      int *getPtr() const { return Ptr; }
+      int *getPtrNonConst() { return PtrNonConst; }
+      int getInt(int i) const { return Int; }
+      int getWithInc(int i) { IntWithInc += i; return IntWithInc; }
+      int getIntNotAccessed() const { return IntNotAccessed; }
       int getIntNoDefinition() const;
+      int &getIntRef() { return IntRef; }
     };
 
     void target() {
       S s;
-      int *p = s.getPtr();
-      int *q = s.getPtrNonConst();
-      int x = s.getInt(1);
-      int y = s.getWithOtherWork(1);
-      int w = s.getIntNoDefinition();
+      int *p1 = s.getPtr();
+      int *p2 = s.getPtrNonConst();
+      int i1 = s.getInt(1);
+      int i2 = s.getWithInc(1);
+      int i3 = s.getIntNoDefinition();
+      int &iref = s.getIntRef();
       // [[p]]
     }
   )";
@@ -1484,10 +1487,11 @@ TEST(TransferTest, StructModeledFieldsWithAccessor) {
         for (auto [Field, _] : SLoc.children())
           Fields.push_back(Field);
         // Only the fields that have simple accessor methods (that have a
-        // single statement body that returns the member variable) should be modeled.
+        // single statement body that returns the member variable) should be
+        // modeled.
         ASSERT_THAT(Fields, UnorderedElementsAre(
-            findValueDecl(ASTCtx, "P"), findValueDecl(ASTCtx, "Q"),
-            findValueDecl(ASTCtx, "X")));
+            findValueDecl(ASTCtx, "Ptr"), findValueDecl(ASTCtx, "PtrNonConst"),
+            findValueDecl(ASTCtx, "Int"), findValueDecl(ASTCtx, "IntRef")));
       });
 }