[PATCH] D50883: [clang-tidy] Handle unique owning smart pointers in ExprMutationAnalyzer

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added inline comments.


================
Comment at: unittests/clang-tidy/ExprMutationAnalyzerTest.cpp:658
+                     "void f() { UniquePtr<const S> x; x->mf(); }");
+  Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+  EXPECT_FALSE(isMutated(Results, AST.get()));
----------------
shuaiwang wrote:
> JonasToth wrote:
> > Template testcases i miss:
> > 
> > ```
> > // modifying
> > template <typename T>
> > void f() { UnqiuePtr<T> x; x->mf(); }
> > 
> > // constant
> > template <typename T>
> > void f2() { UnqiuePtr<T> x; x->cmf(); }
> > 
> > // indecidable for the template itself, but only the instantiations
> > template <typename T>
> > void f3() { T x; x->cmf(); }
> > 
> > struct const_class { void cmf() const; }
> > struct modifying_class { void cmf(); };
> > 
> > void call_template() {
> > // don't trigger
> > f3<UniquePtr<const_class>>();
> > // trigger modification
> > f3<random_class*>();
> > }
> > 
> > // again not decidable by the template itself
> > template <typename T>
> > void f4() { T t; *t; }
> > 
> > struct very_weird {
> >     int& operator*() { return *new int(42); }
> > };
> > void call_template_deref() {
> >   // no modification
> >   f4<int*>();
> >   // modification, because deref is not const
> >   f4<UniquePtr<very_weird>>():
> > }
> > ```
> Added a case with template. However I don't think we can do much whenever template appears: only the AST of **uninstantiated** template worth analyzing, because whatever analyze result (diag or fixit) would have to be applied to the template itself not the instantiations.
> So the fact that the template argument of `f3` or `f4` could happen to be `UniquePtr` doesn't really matter when we analyze `f3` and `f4`, we have to analyze the way assuming the "worst" anyway.
Yes, whenever there is something type-dependent we have to bail, thats why i would like to see the testcases to show that we actually bail, even when one instantiation would not modify.

I believe we do analyze all versions of the templated functions (uninstantiated and every instantiation), don't we? With that there can be conflicting results.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D50883