[PATCH] D50619: [clang-tidy] Handle unresolved expressions in ExprMutationAnalyzer

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added inline comments.


================
Comment at: unittests/clang-tidy/ExprMutationAnalyzerTest.cpp:117
 
+TEST(ExprMutationAnalyzerTest, AssumedNonConstMemberFunc) {
+  auto AST = tooling::buildASTFromCode(
----------------
I think you could add another test with `X<T> x` (if you don't have it already and I overlooked them)


================
Comment at: unittests/clang-tidy/ExprMutationAnalyzerTest.cpp:309
 
+TEST(ExprMutationAnalyzerTest, CallUnresolved) {
+  auto AST =
----------------
I think we are missing tests for non-type template paramters (`template <size_t N>`). They should behave the same. But the following case would not be a mutation:

```
void non_mutating(const char* Array, int size) { /* Foo */ }
template <int N>
struct ArrayLike {
  char* data[N]; // Initialized to something
  void SomeFunction() {
    non_mutating(data, N);
  }
};
```

The difference between the 'normal' and non-type templates would be, that `N` is not mutatable at all and the semantics is clear (builtin integer-like type).

If the current implementation would not figure that out, you can just add a test for it and assume a mutation. Handling non-type templates later is absolutly ok.


================
Comment at: unittests/clang-tidy/ExprMutationAnalyzerTest.cpp:410
+      match(withEnclosingCompound(declRefTo("y")), AST->getASTContext());
+  EXPECT_THAT(mutatedBy(ResultsY, AST.get()), ElementsAre("y"));
+}
----------------
Out of curiosity: Why is the result with `y` different from the result for `x`? Both time `x` is mutated and `g()` mutates them.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D50619