[PATCH] D104288: [VP][NFCI] Address various clang-tidy warnings

[Simon Moll via Phabricator via llvm-commits]

simoll added inline comments.


================
Comment at: llvm/unittests/IR/VPIntrinsicTest.cpp:253
     ASSERT_EQ(F.getIntrinsicID(), NewDecl->getIntrinsicID());
-    auto ItNewParams = NewDecl->getFunctionType()->param_begin();
-    auto EndItNewParams = NewDecl->getFunctionType()->param_end();
+    const auto *ItNewParams = NewDecl->getFunctionType()->param_begin();
+    const auto *EndItNewParams = NewDecl->getFunctionType()->param_end();
----------------
frasercrmck wrote:
> simoll wrote:
> > I know `const auto*` follows the convention. However, i like just `const auto` here because do we really want to assert the type structure of what is actually just 'some' iterator to us?
> Yes I agree it's a bit weird because it looks like an iterator but it's ultimately a `Type * const *`. Alternatively we could perhaps explicitly use `FunctionType::param_iterator`. That's also a fairly common way of doing it in llvm, relatively speaking. I'm a bit wary of overriding clang-tidy in general as its opinions -- rightly or wrongly -- are what decides whether tools like editors show warnings, whether patches are landed "despite ongoing or failed builds" etc.
+1 for `FunctionType::param_iterator`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104288