aaron.ballman added a comment.
I reviewed the changes in the patch and they seem reasonable, but this patch is
hard to have high confidence in because you need to audit all the places where
the default behavior silently changed and no changes were made in the patch.
I'm assuming that the code changes were made to cause existing tests to pass,
but did you verify that changes weren't needed to checks that show no
behavioral test changes to ensure that it wasn't because the tests have poor
template test coverage?
================
Comment at:
clang-tools-extra/clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp:62
StatementMatcher LoopVarConversionMatcher =
traverse(ast_type_traits::TK_AsIs,
implicitCastExpr(hasImplicitDestinationType(isInteger()),
----------------
Do we still need this `TK_AsIs` with the below changes?
================
Comment at: clang/lib/ASTMatchers/ASTMatchFinder.cpp:564
+ bool TraversingImplicitTemplateInstantiation = false;
+
----------------
This should not be part of the public interface.
================
Comment at: clang/lib/ASTMatchers/ASTMatchFinder.cpp:570
+
+ struct ImplicitTemplateInstantiationScope {
+ ImplicitTemplateInstantiationScope(MatchASTVisitor *V, bool B)
----------------
Nor should this, right?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D80961/new/
https://reviews.llvm.org/D80961
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
