This is an archive of the discontinued LLVM Phabricator instance.

[clang-tidy] Handle sugared reference types in ExprMutationAnalyzer
ClosedPublic

Authored by shuaiwang on Aug 19 2018, 9:52 PM.

Details

Summary

This handles cases like this:

typedef int& IntRef;
void mutate(IntRef);
void f() {
  int x;
  mutate(x);
}

where the param type is a sugared type (TypedefType) instead of a
reference type directly.

Note that another category of similar but different cases are already
handled properly before:

typedef int Int;
void mutate(Int&);
void f() {
  int x;
  mutate(x);
}

Diff Detail

Repository
rL LLVM

Event Timeline

shuaiwang created this revision.Aug 19 2018, 9:52 PM

What happens to pointers in a typedef (in the sense of * instead of &)?

clang-tidy/utils/ExprMutationAnalyzer.cpp
51 ↗(On Diff #161415)

Not directly related, but this matcher matches universal references, even though they might result in a value. (one of the bugs lebedevri reported).

unittests/clang-tidy/ExprMutationAnalyzerTest.cpp
171 ↗(On Diff #161415)

i think the following tests should be added as well

  • typedef const int& IntRef;
  • template <typename T> using TemplateRef = T&

It would be better to have at least one positive and negative test (with const and without const in the typedef) for each of theses cases.

shuaiwang marked 2 inline comments as done.

more test cases.

What happens to pointers in a typedef (in the sense of * instead of &)?

I checked around and I believe reference type is the only type we're explicitly matching right now. We'll need to handle carefully when handling pointer types in the future.

clang-tidy/utils/ExprMutationAnalyzer.cpp
51 ↗(On Diff #161415)

Noted, I'll take a look at the bug.

What happens to pointers in a typedef (in the sense of * instead of &)?

I checked around and I believe reference type is the only type we're explicitly matching right now. We'll need to handle carefully when handling pointer types in the future.

We match on pointers as values. So figure this out int * >const< ptr = nullptr.
And the constness transformation is especially intersting for pointer typedefs, because in typedef int * IntPtr; const IntPtr p1; IntPtr const p2; p1 and p2 are different things. It would be nice if you could check, that the value semantic of the pointer is detected through the typedef as well.

more test cases

Could you please add a mutating test for the pointers as well?

What happens to pointers in a typedef (in the sense of * instead of &)?

I checked around and I believe reference type is the only type we're explicitly matching right now. We'll need to handle carefully when handling pointer types in the future.

We match on pointers as values. So figure this out int * >const< ptr = nullptr.
And the constness transformation is especially intersting for pointer typedefs, because in typedef int * IntPtr; const IntPtr p1; IntPtr const p2; p1 and p2 are different things. It would be nice if you could check, that the value semantic of the pointer is detected through the typedef as well.

Added test cases verifying we're treating pointers as values.
I feel constness doesn't matter much since we're treating them as values and both const values & non-const values are just values.

I feel constness doesn't matter much since we're treating them as values and both const values & non-const values are just values.

marking the pointer itself const is not very common as well, thats why i feel we should be especially exhaustive with our tests as real projects probably won't show problems.

More test cases:

  • Mutating pointers
  • Const values
This revision is now accepted and ready to land.Sep 11 2018, 11:55 AM
This revision was automatically updated to reflect the committed changes.