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

Valeriy Savchenko via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Feb 3 06:01:41 PST 2021


vsavchenko added a comment.

Also, all these `PathRange` and `PathList` don't really reflect what they stand for semantically and talk more about implementation.  It's not the problem of this patch, but it doesn't make your logic easier to comprehend.



================
Comment at: clang/lib/StaticAnalyzer/Core/BasicValueFactory.cpp:226
+        llvm::make_range(BaseList.begin(), BaseList.end()),
+        [](const auto &I) -> bool { return I.second; });
+
----------------
I think it's all a bit too complicated now.

Right now you have two nested loops:
  1. over `PathRange`
  2. over `BaseList` (in `find_if`)

It won't be a problem to put them in another order, so when you make a `filter_range`, you can have `llvm::find_if` which will iterate over `PathRange` and captures the given `BaseList` element. So, yes nested lambdas.

Here is a sketch of the solution:
```
auto BaseListFilteredRange = llvm::make_filter_range(
    BaseList, // you don't need to make_range here, BaseList IS a range
    [&PathRange](const CXXBaseSpecifier *Base) {
      return llvm::find_if(PathRange, [Base](const auto &It) {
        return Base->getType() == It.first->getType();
      }) == PathRange.end();
    });
```

Aaaand while I was writing that, it got me thinking that we don't need `BaseList` in the first place:
```
for (const CXXBaseSpecifier *Base : PathList)
  if (llvm::none_of(PathRange,
                    [Base](const auto &It) { return Base->getType() == It.first->getType(); })
      PathList = CXXBaseListFactory.add(Base, PathList);
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95877



More information about the cfe-commits mailing list