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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits