[PATCH] D95877: [analyzer] Fix static_cast on pointer-to-member handling

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov added a comment.

I downloaded and tested this patch. On the whole it works OK. See my suggestions below.



================
Comment at: clang/lib/StaticAnalyzer/Core/BasicValueFactory.cpp:205
+  llvm::SmallPtrSet<QualType, 16> BaseSpecSeen;
+  for (const auto &BaseSpec : BaseSpecList) {
+    auto BaseType = BaseSpec->getType();
----------------
I suggest to use explicit types instead of `auto` to increase readability.


================
Comment at: clang/lib/StaticAnalyzer/Core/BasicValueFactory.cpp:226-229
+      if (llvm::none_of(
+              PathRange, [&BaseSpec](const auto &I) -> auto {
+                return BaseSpec->getType() == I->getType();
+              }))
----------------
It will look clearer if we use an intermediate variable for predicate here.


================
Comment at: clang/lib/StaticAnalyzer/Core/BasicValueFactory.cpp:234-238
+  } else {
+    for (const auto &I : llvm::reverse(PathRange))
+      BaseSpecList = prependCXXBase(I, BaseSpecList);
+    return getPointerToMemberData(ND, BaseSpecList);
+  }
----------------
We can remove else branch here and move its body out unconditionally as the main one already has `return` at the end.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95877