vsavchenko added inline comments.
================
Comment at: clang/lib/StaticAnalyzer/Core/BasicValueFactory.cpp:180
+bool noRepeatedElements(
+ const llvm::ImmutableList<const CXXBaseSpecifier *> &BaseSpecList) {
----------------
nit: functions represent actions, in languages verbs act the same way, so it's
recommended to use verbs in functions (`hasNoRepeatedElements`)
+ `LLVM_MAYBE_UNUSED` because in the build without assertions it would be a
warning otherwise.
================
Comment at: clang/lib/StaticAnalyzer/Core/BasicValueFactory.cpp:181
+bool noRepeatedElements(
+ const llvm::ImmutableList<const CXXBaseSpecifier *> &BaseSpecList) {
+ // First we need to make sure that there are no-repetitions in BaseSpecList
----------------
`ImmutableList` is one of those value types that prefers copying for clarity.
================
Comment at: clang/lib/StaticAnalyzer/Core/BasicValueFactory.cpp:184
+ llvm::SmallPtrSet<QualType, 16> BaseSpecSeen;
+ for (const auto &BaseSpec : BaseSpecList) {
+ auto BaseType = BaseSpec->getType();
----------------
LLVM Coding Style calls for very limited use of `auto`, so here and the next
line would be better with actual types.
================
Comment at: clang/lib/StaticAnalyzer/Core/BasicValueFactory.cpp:184
+ llvm::SmallPtrSet<QualType, 16> BaseSpecSeen;
+ for (const auto &BaseSpec : BaseSpecList) {
+ auto BaseType = BaseSpec->getType();
----------------
vsavchenko wrote:
> LLVM Coding Style calls for very limited use of `auto`, so here and the next
> line would be better with actual types.
Let's also reiterate on using functional-style predicates and reimplement it in
terms of `llvm::all_of` or `llvm::none_of`.
================
Comment at: clang/lib/StaticAnalyzer/Core/BasicValueFactory.cpp:232
+ auto ReducedBaseSpecList = CXXBaseListFactory.getEmptyList();
+ for (const auto &BaseSpec : BaseSpecList)
+ if (llvm::none_of(
----------------
The same thing about `auto`
================
Comment at: clang/lib/StaticAnalyzer/Core/BasicValueFactory.cpp:234
+ if (llvm::none_of(
+ PathRange, [&BaseSpec](const auto &I) -> auto {
+ return BaseSpec->getType() == I->getType();
----------------
No need
================
Comment at: clang/test/Analysis/pointer-to-member.cpp:314-333
+// namespace testReinterpretCasting {
+// struct Base {
+// int field;
+// };
+//
+// struct Derived : public Base {};
+//
----------------
Uncomment it, and expect the actual current result. This is where `TODO` will
come in handy.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D95877/new/
https://reviews.llvm.org/D95877
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
