royjacobson marked 2 inline comments as done. royjacobson added a comment. In D135404#3842156 <https://reviews.llvm.org/D135404#3842156>, @njames93 wrote:
> Why limit this to just the value type traits, makes sense to also support > type type traits. I'd love to, but I can't match against the `::type` usages with `DeclRefExpr`. I think I'd have to match all types in the program or something like that, but I'm not sure if even that's enough because of unevaluated contexts etc... So I decided to focus on this first. ================ Comment at: clang-tools-extra/clang-tidy/modernize/TypeTraitsCheck.cpp:73 + const auto *RecordQualifier = + cast_if_present<ClassTemplateSpecializationDecl>( + Qualifier->getAsRecordDecl()); ---------------- njames93 wrote: > Won't this assert if a pointer is supplied but it isn't a > ClassTemplateSpecializationDefl. Wouldn't dyn_cast be needed. > In any case a test where you access a static variable from a non template > class needs adding to make sure no warning is emitter. Definitely need a dyn_cast here, thanks! ================ Comment at: clang-tools-extra/clang-tidy/modernize/TypeTraitsCheck.cpp:86 + auto CurrentText = tooling::fixit::getText(*MatchedDecl, *Result.Context); + if (replacement_regex.match(CurrentText)) + IssuedDiag << FixItHint::CreateReplacement( ---------------- njames93 wrote: > This fixit logic looks cumbersome, just replace the qualifier loc(::)to the > declared with an `_v`. But only iff the qualified name loc is not a macro. I think to do that I have to get the location of the template-id of the qualifier from the ClassTemplateSpecializationDecl, but I haven't really managed to do that. Or is there maybe a way to get the lexer tokens list of a source range or something like that? ================ Comment at: clang-tools-extra/docs/clang-tidy/checks/modernize/type-traits.rst:28 + +.. option:: ValueTypeTraits + ---------------- njames93 wrote: > I'm not sure this option would ever be used in the wild as it would override > all the default values in the stl. The intended use case is one-time running of this check over a code base to modernize it, and you want to add your own type traits that you define across your own libraries. I think that's useful, at least. Maybe this should just _add_ traits, and not replace them? Is this something that clang-tidy does? I basically copied this from the `modernize-use-emplace` check. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135404/new/ https://reviews.llvm.org/D135404 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
