aaron.ballman added inline comments.
================
Comment at: clang/lib/Analysis/ExprMutationAnalyzer.cpp:44
+ InnerMatcher) {
+ // Unless the value is a derived class and is assigned to a
+ // reference to the base class. Other implicit casts should not
----------------
Unless the value -> Matches unless the value
================
Comment at: clang/lib/Analysis/ExprMutationAnalyzer.cpp:65
+// not have the 'arguments()' method.
+AST_MATCHER_P(InitListExpr, hasAnyArgumentExpr,
+ ast_matchers::internal::Matcher<Expr>, InnerMatcher) {
----------------
I think I'd prefer this to be named `hasAnyInit()` to complement `hasInit()` --
these aren't really arguments.
================
Comment at: clang/lib/Analysis/ExprMutationAnalyzer.cpp:262
+ hasArgument(0, ignoringImpCasts(canResolveToExpr(equalsNode(Exp))))),
+ // operator call expression might be unresolved as well. If that is
+ // the case and the operator is called on the 'Exp' itself, this is
----------------
operator call expression -> The call operator expression
================
Comment at: clang/lib/Analysis/ExprMutationAnalyzer.cpp:264
+ // the case and the operator is called on the 'Exp' itself, this is
+ // considered a moditication.
+ cxxOperatorCallExpr(
----------------
moditication -> modification
================
Comment at: clang/lib/Analysis/ExprMutationAnalyzer.cpp:326
+ // If the initializer is for a reference type, there is no cast for
+ // the variable. Values are casted to RValue first.
+ initListExpr(
----------------
casted -> cast
================
Comment at: clang/lib/Analysis/ExprMutationAnalyzer.cpp:449
+
+ // It is possible, that containers do not provide a const-overload for their
+ // iterator accessors. If this is the case, the variable is used non-const
----------------
It is possible, that -> It is possible that
================
Comment at: clang/lib/Analysis/ExprMutationAnalyzer.cpp:452
+ // no matter what happens in the loop. This requires special detection as it
+ // is faster to find then all mutations of the loop variable.
+ // It aims at a different modification as well.
----------------
is faster to find then all -> is then faster to find all
================
Comment at: clang/lib/Analysis/ExprMutationAnalyzer.cpp:455
+ const auto HasAnyNonConstIterator =
+ anyOf(allOf(hasMethod(allOf(hasName("begin"), unless(isConst()))),
+ unless(hasMethod(allOf(hasName("begin"), isConst())))),
----------------
Do we want to look for methods that end with `_?[Bb]egin` or `_?[Ee]nd` so that
this would catch patterns like `foo_begin()`/`foo_end()`,
`FooBegin()`/`FooEnd()`, or `Foo_Begin()`/`Foo_End()`?
================
Comment at: clang/unittests/Analysis/ExprMutationAnalyzerTest.cpp:65
+
+ std::string buffer;
for (const auto *E = selectFirst<Expr>("expr", Results); E != nullptr;) {
----------------
Was there a reason you hoisted this out of the `for` loop?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D88088/new/
https://reviews.llvm.org/D88088
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
