[PATCH] D25316: [clang-tidy] Fix PR25499: Enhance modernize-use-auto to casts

[Piotr Padlewski via cfe-commits]

Prazek added a comment.

In https://reviews.llvm.org/D25316#565463, @malcolm.parsons wrote:

> In https://reviews.llvm.org/D25316#565378, @Prazek wrote:
>
> > Awesome to see this patch. After this one will make it to upstream, it will be much easier for me to do same with template functions.
>
>
> I was trying to match such functions:
>
>   varDecl(hasType(type().bind("type")),
>           hasInitializer(callExpr(callee(
>               functionDecl(isInstantiated(),
>                            hasTemplateArgument(
>                                0, refersToType(type(equalsBoundNode("type")))))
>                   .bind("fn")))))


I did some implementation long ago for boost-lexical-last here: https://reviews.llvm.org/D17765
My matchers were:

  Finder->addMatcher(
      declStmt(has(varDecl(hasInitializer(callExpr(callee(
                               functionDecl(hasName(LEXICAL_CAST_NAME))))),
                           unless(hasType(autoType())))))
          .bind("same_type"),
      this);
  
  Finder->addMatcher(
      declStmt(has(varDecl(hasInitializer(implicitCastExpr(has(callExpr(
                   callee(functionDecl(hasName(LEXICAL_CAST_NAME))))))))))
          .bind("different_type"),
      this);



================
Comment at: test/clang-tidy/modernize-use-auto-cast-remove-stars.cpp:2
+// RUN: %check_clang_tidy %s modernize-use-auto %t -- \
+// RUN:   -config="{CheckOptions: [{key: modernize-use-auto.RemoveStars, value: '1'}]}" \
+// RUN:   -- -std=c++11
----------------
malcolm.parsons wrote:
> Prazek wrote:
> > What is the difference between this test, and next test?
> > The name indicate that it removes star, but I see a lot of test that doesn't use star
> > and that seem to be duplicated with the next one.
> I could remove the duplicated tests, but the expected fixes are different so I'd like to test with and without star removal.
so I think the best thing to do here, is to merge 2 files together, add second RUN: with different -check-prefix and use this prefix in the tests. At least this is how the tests in LLVM works, not sure if this is implemented in check_clang_tidy. 


================
Comment at: test/clang-tidy/modernize-use-auto-cast-remove-stars.cpp:25
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use auto when initializing with a cast to avoid duplicating the type name
+  // CHECK-FIXES: auto  ll = static_cast<long long>(l);
+  unsigned long long ull = static_cast<unsigned long long>(l);
----------------
malcolm.parsons wrote:
> Prazek wrote:
> > Is it possible to simply fix the double spaces?
> The existing modernize-use-auto tests have the same problem.
> My codebase is clang-formatted regularly so it doesn't bother me.
I agree that it is not huge problem with clang-format and of course it is not required to fix it to push this patch upstream. It would be good to leave a comment somewhere with FIXME: about this, so it would be easier for some other contributor to fix it (or if it simple, just fix it in this patch :))


https://reviews.llvm.org/D25316