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

[Fraser Cormack via Phabricator via llvm-commits]

frasercrmck marked an inline comment as done.
frasercrmck 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();
----------------
simoll wrote:
> 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`
You have yourself a deal! I take it your LGTM still stands? I'll wait for your okay.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104288