[PATCH] D21036: Misplaced const-qualification with typedef types

[Aaron Ballman via cfe-commits]

aaron.ballman marked an inline comment as done.
aaron.ballman added a comment.

Thank you for the quick turn-around!


================
Comment at: test/clang-tidy/misc-misplaced-const.c:20
@@ +19,3 @@
+  const volatile ip i4 = 0;
+  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: 'i4' declared with a const-qualified typedef type; results in the type being 'int *const volatile' instead of 'const int *volatile'
+}
----------------
sbenza wrote:
> sbenza wrote:
> > If we are guessing that 'const' goes to the int, shouldn't we guess that volatile also goes to the int?
> Only keep the first instance with the full message.
> The rest can be collapsed with {{.*}} to try to fit in 80 cols.
Actually, that test was purposely showing that we do *not* treat all cv-qualifiers the same. `volatile` and `restrict` have sufficiently complex semantics that I would be wary about pointing out the type differences as being potentially problematic. Typically, if you are using those qualifiers, you probably know more about what you want the code to do and whether you want the *pointer* to have the qualifier instead of the pointee.

================
Comment at: test/clang-tidy/misc-misplaced-const.c:20
@@ +19,3 @@
+  const volatile ip i4 = 0;
+  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: 'i4' declared with a const-qualified typedef type; results in the type being 'int *const volatile' instead of 'const int *volatile'
+}
----------------
aaron.ballman wrote:
> sbenza wrote:
> > sbenza wrote:
> > > If we are guessing that 'const' goes to the int, shouldn't we guess that volatile also goes to the int?
> > Only keep the first instance with the full message.
> > The rest can be collapsed with {{.*}} to try to fit in 80 cols.
> Actually, that test was purposely showing that we do *not* treat all cv-qualifiers the same. `volatile` and `restrict` have sufficiently complex semantics that I would be wary about pointing out the type differences as being potentially problematic. Typically, if you are using those qualifiers, you probably know more about what you want the code to do and whether you want the *pointer* to have the qualifier instead of the pointee.
I'll collapse the ones after the volatile test.

================
Comment at: test/clang-tidy/misc-misplaced-const.cpp:20
@@ +19,3 @@
+
+template <typename Ty>
+struct S {
----------------
sbenza wrote:
> I assume this check does not trigger for 'const X&' where X is a pointer.
> Please add a test for that.
Do you want:
```
template <typename Ty>
struct S {
  const Ty *i;
  const Ty &i2;
};

template struct S<int>;
template struct S<ip>; // ok
template struct S<cip>;
template struct S<int *>; // ok
```
Or something else?


http://reviews.llvm.org/D21036