JonasToth added a comment.
LGTM, with a few nits and a question. Please wait a bit if @aaron.ballman or
@george.karpenkov have any comments before committing.
================
Comment at: lib/Analysis/ExprMutationAnalyzer.cpp:88
+bool isPointeeMutable(const Expr *Exp, const ASTContext &Context) {
+ if (const auto *PT = Exp->getType()->getAs<PointerType>()) {
+ return !PT->getPointeeType().isConstant(Context);
----------------
you can elide the braces here
================
Comment at: lib/Analysis/ExprMutationAnalyzer.cpp:101
+ return false;
+ // Check whether returning non-const reference.
+ if (const auto *RT =
----------------
Please make this a full sentence (adding `... the overloaded 'operator*' is
returning ...` is enough.)
================
Comment at: lib/Analysis/ExprMutationAnalyzer.cpp:227
+ // `Exp` can be *directly* mutated if the type of `Exp` is not const.
+ // Bail out early otherwise.
+ if (Exp->getType().isConstant(Context))
----------------
This comment is one sentence, just replace `const. Bail` with `const, bail`
================
Comment at: unittests/Analysis/ExprMutationAnalyzerTest.cpp:87
+ EXPECT_TRUE(isPointeeMutated(Results, AST));
+ const auto *const S = selectFirst<Stmt>("stmt", Results);
+ const auto *const E = selectFirst<Expr>("expr", Results);
----------------
pointer is `const`, please remove the last `const`
================
Comment at: unittests/Analysis/ExprMutationAnalyzerTest.cpp:645
+ // TODO: this should work when casts of pointers are handled.
+ // EXPECT_EQ(pointeeMutatedBy(Results, AST.get()), "return x;");
----------------
I am confused. The `const int*` guarantees, that `x`-pointee is not mutated.
Why shall this be diagnosed as a mutation?
Repository:
rC Clang
https://reviews.llvm.org/D52219
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
