[PATCH] D54395: [clang-tidy] implement utility-function to add 'const' to variables

[Jonas Toth via Phabricator via cfe-commits]

JonasToth marked 5 inline comments as done.
JonasToth added inline comments.
Herald added a subscriber: mgehre.


================
Comment at: clang-tidy/utils/FixItHintUtils.cpp:35
+static bool isValueType(QualType QT) { return isValueType(QT.getTypePtr()); }
+static bool isArrayType(QualType QT) { return isa<ArrayType>(QT.getTypePtr()); }
+static bool isReferenceType(QualType QT) {
----------------
lebedev.ri wrote:
> JonasToth wrote:
> > aaron.ballman wrote:
> > > This file is replicating a bunch of functionality that exists on the `Type*` already. Why do this rather than have the caller do `QT->isArrayType()` and skip this function entirely? (Same exists elsewhere here.)
> > I had the issue that typedefs are resolved, but shouldn't. If I am not mistaken `QT->isArrayType()` would go to the canconical type.
> > 
> > ```
> > using MyPtr = int*;
> > MyPtr foo = nullptr;
> > ```
> > Is treated wrong in that case.
> There is a test for that, right?
Yes, both `typedef` and `using`.


================
Comment at: unittests/clang-tidy/AddConstTest.cpp:733
+  StringRef T = "template <typename T> void f(T v) \n";
+  StringRef S = "{ T target = v; }";
+  auto Cat = [&T](StringRef S) { return (T + S).str(); };
----------------
lebedev.ri wrote:
> lebedev.ri wrote:
> > JonasToth wrote:
> > > alexfh wrote:
> > > > It would be interesting to see test cases with multiple instantiations of the template the fix applies to.
> > > I added test for a template function with many instantiations, but there should not be a difference between the instantiations, because only the original code would be transformed, and there the 'how it looks' counts, so in this case it will be treated as a value.
> > > Did I misinterpret your question?
> > How about https://en.cppreference.com/w/cpp/language/function_template#Explicit_instantiation ?
> > I don't see any tests for that.
> Also, is there any test coverage missing for 
> https://en.cppreference.com/w/cpp/language/template_specialization
> and
> https://en.cppreference.com/w/cpp/language/partial_specialization
> ?
i added tests for specializations.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D54395