[clang] Implement -Wcast-qual for C++

[lebedev.ri]

Summary

This way, the behavior of that warning flag is more closely resembles that of GCC.

Fixes PR4802.

Testing:

ninja check-clang-sema check-clang-semacxx

Do note that there is at least one false-negative (see FIXME in tests).

[lebedev.ri]

Hm, now that i think about it, i should also add c++-specific tests too.

[lebedev.ri]

Add C++-specific tests, handle C++ `*_cast<>()`'s too.
This does warn on const_cast, as it should.

FIXME: c++ reference handling.

[rnk]

Looks good, do you need someone to commit this? You might also consider requesting commit access: http://llvm.org/docs/DeveloperPolicy.html#obtaining-commit-access

test/SemaCXX/warn-cast-qual.cpp

38

Maybe move the FIXMEs to the lines that should warn but don't? The (int &) lines seem relevant here.

[lebedev.ri]

In D33102#753945, @rnk wrote:

Looks good

No, it's not. First, it still contains "WIP" in the subject.
After i have last updated this revision, i talked to David in IRC, and we agreed that it actually makes no sense to diagnose const_cast<>(), that is clang-tidy's cppcoreguidelines-pro-type-const-cast job.
I did change that locally, but did not upload new revision yet.
And, i'll need to add c++ references handling too.
Should be a bit more time.

(i shouldn't have even opened this revision until it's ready, but only i have opened it, i have thought about the c++ references/etc)

, do you need someone to commit this? You might also consider requesting commit access: http://llvm.org/docs/DeveloperPolicy.html#obtaining-commit-access

[lebedev.ri]

Do not diagnose const_cast<>(),
Do handle C++ references.
Add more tests - c++ references, class functions, c/c++ struct members

I think this is it, at least i'm not sure what else to test.
Please do review carefully!

@ EVERYONE: please do note that this essentially implements entirely new warning for C++ !
Even though that warning existed before; but in clang (unlike gcc), it did nothing in C++.

[dblaikie]

That seems like way more tests than would be ideal - perhaps it can be reduced substantially by considering equivalence classes?

(for example, I wouldn't expect more than one test case for C++ const_casts - if the code simply says "if this is a const_cast, don't do anything" then the test should be pretty similar in that regard - if there were more nuance in the selection of which const_casts to warn on, I'd expect more granularity in the testing there - but no need to test all the variations of const_cast that don't warn because you're testing those same variations of c-style cast)

lib/Sema/SemaCast.cpp

2603

do you need to test langopts here? Are there cases where "isLValueReferenceType" would return true and it wasn't CPlusPlus? And are they cases that need to be suppressed/ignored?

[lebedev.ri]

Do not check that we are in C++ before checking that dest type is a lvalue reference.
Trim SemaCXX testcase a little.

[lebedev.ri]

lib/Sema/SemaCast.cpp

2603

Good question.
No, i don't think that is needed at all, at least not that i'm aware of.
I'll remove that.

[alexfh]

test/SemaCXX/warn-cast-qual.cpp

47

Should this be cast from 'const int&' ...? (note the missing &)

[lebedev.ri]

test/SemaCXX/warn-cast-qual.cpp

47

Hmm, that may be more informative indeed

gcc output:

$ g++ -Wcast-qual  llvm/tools/clang/test/SemaCXX/warn-cast-qual.cpp 
warn-cast-qual.cpp: In function ‘void foo_0()’:
warn-cast-qual.cpp:46:20: warning: cast from type ‘const int*’ to type ‘int*’ casts away qualifiers [-Wcast-qual]
   int &a2 = (int &)a;                      // expected-warning {{cast from 'const int' to 'int &' drops const qualifier}}
                    ^

Any other opinions?

[lebedev.ri]

test/SemaCXX/warn-cast-qual.cpp

47

Upon further considerations,

• a is const int, not, const int&, so the warning would not refer to the original a, but to the automatically-taken reference to a (even though it is exactly the same thing as a)
• if a would be const int&, c++ standard, [expr].5 says:

5 If an expression initially has the type “reference to T” (8.3.2, 8.5.3), the type is adjusted to T prior to
any further analysis.

So this may actually be better to always leave out the &.
I.e. i think it should stay as-is. But i'm no expert on the subject.

[lebedev.ri]

Ping :)

[alexfh]

test/SemaCXX/warn-cast-qual.cpp

47

I.e. i think it should stay as-is. But i'm no expert on the subject.

Neither am I. But your arguments seem reasonable. SGTM.

[lebedev.ri]

Friendly weekly ping :)

[dblaikie]

Still seems like an awful lot more testing than I'd expect for this warning. (eg: testing all the many variations of C++ const_cast - when they all provide basically the same coverage I think, that all const_casts don't warn)

lib/Sema/SemaCast.cpp

483–495

Avoid else after return, instead write:

if (...)
  return ...
if (...)
  return ...

etc...

[lebedev.ri]

Address review notes.

[lebedev.ri]

In D33102#767291, @dblaikie wrote:

Still seems like an awful lot more testing than I'd expect for this warning. (eg: testing all the many variations of C++ const_cast - when they all provide basically the same coverage I think, that all const_casts don't warn)

Ok, i removed a bit more of const_cast checks, but unless you insist, i'd think all the rest of the test are needed :)
I do understand that it is ideal to keep the checks as small as possible for speed reasons.

[dblaikie]

I still feel like that's more testing than would be ideal (does the context of the cast matter? Wether it's dereferenced, a struct member access, assigned, initialized, etc - it doesn't look like it from the code, etc).

But sure. Could you also (manually, I guess) confirm that this matches GCC's cast-qual behavior (insofar as the warning fires in the same situations). If there are any deviations, let's chat about them.

[lebedev.ri]

In D33102#773296, @dblaikie wrote:

I still feel like that's more testing than would be ideal (does the context of the cast matter? Wether it's dereferenced, a struct member access, assigned, initialized, etc - it doesn't look like it from the code, etc).

Looking at the CastsAwayConstness() function in lib/Sema/SemaCast.cpp: https://github.com/llvm-mirror/clang/blob/432ed0e4a6d58f7dda8992a167aad43bc91f76c6/lib/Sema/SemaCast.cpp#L505-L510
You can see that it asserts that the pointer is one of three types. So i think it it is best to have maybe slightly overlapping test coverage here, rather than be surprised one day that such trivial cases no longer warn...

But sure. Could you also (manually, I guess) confirm that this matches GCC's cast-qual behavior (insofar as the warning fires in the same situations). If there are any deviations, let's chat about them.

Sure.

1. Gcc produces the same count of the warnings:

$ pwd
llvm/tools/clang/test
$ grep -o "expected-warning" Sema/warn-cast-qual.c | wc -l
14
$ gcc -x c -fsyntax-only -Wcast-qual -c Sema/warn-cast-qual.c 2>&1 | grep ": warning: " | wc -l
14
$ gcc -x c++ -fsyntax-only -Wcast-qual -c Sema/warn-cast-qual.c 2>&1 | grep ": warning: " | wc -l
14
$ grep -o "expected-warning" SemaCXX/warn-cast-qual.cpp | wc -l
39
$ gcc -x c++ -fsyntax-only -Wcast-qual -c SemaCXX/warn-cast-qual.cpp 2>&1 | grep ": warning: " | wc -l
39

2. I'm not quite sure how to non-manually compare the warnings, so i'll just show the gcc output on these three cases. Since the clang warnings are appended as comments at the end of the each line that should warn, visual comparison is possible:

2.1.

$ gcc -x c -fsyntax-only -Wcast-qual -c Sema/warn-cast-qual.c
Sema/warn-cast-qual.c: In function ‘foo’:
Sema/warn-cast-qual.c:9:13: warning: cast discards ‘const’ qualifier from pointer target type [-Wcast-qual]
   char *y = (char *)ptr; // expected-warning {{cast from 'const char *' to 'char *' drops const qualifier}}
             ^
Sema/warn-cast-qual.c:10:15: warning: cast discards ‘const’ qualifier from pointer target type [-Wcast-qual]
   char **y1 = (char **)ptrptr; // expected-warning {{cast from 'const char *const' to 'char *' drops const qualifier}}
               ^
Sema/warn-cast-qual.c:11:21: warning: cast discards ‘const’ qualifier from pointer target type [-Wcast-qual]
   const char **y2 = (const char **)ptrptr; // expected-warning {{cast from 'const char *const *' to 'const char **' drops const qualifier}}
                     ^
Sema/warn-cast-qual.c:14:14: warning: cast discards ‘const’ qualifier from pointer target type [-Wcast-qual]
   char *z1 = (char *)(const void *)ptr; // expected-warning {{cast from 'const void *' to 'char *' drops const qualifier}}
              ^
Sema/warn-cast-qual.c:17:16: warning: cast discards ‘volatile’ qualifier from pointer target type [-Wcast-qual]
   char *vol2 = (char *)vol; // expected-warning {{cast from 'volatile char *' to 'char *' drops volatile qualifier}}
                ^
Sema/warn-cast-qual.c:19:17: warning: cast discards ‘const volatile’ qualifier from pointer target type [-Wcast-qual]
   char *volc2 = (char *)volc; // expected-warning {{cast from 'const volatile char *' to 'char *' drops const and volatile qualifiers}}
                 ^
Sema/warn-cast-qual.c:22:28: warning: to be safe all intermediate pointers in cast from ‘int **’ to ‘const int **’ must be ‘const’ qualified [-Wcast-qual]
   const int **intptrptrc = (const int **)intptrptr; // expected-warning {{cast from 'int **' to 'const int **' must have all intermediate pointers const qualified}}
                            ^
Sema/warn-cast-qual.c:23:31: warning: to be safe all intermediate pointers in cast from ‘int **’ to ‘volatile int **’ must be ‘const’ qualified [-Wcast-qual]
   volatile int **intptrptrv = (volatile int **)intptrptr; // expected-warning {{cast from 'int **' to 'volatile int **' must have all intermediate pointers const qualified}}
                               ^
Sema/warn-cast-qual.c:29:23: warning: cast discards ‘const’ qualifier from pointer target type [-Wcast-qual]
   char **charptrptr = (char **)charptrptrc; // expected-warning {{cast from 'const char *' to 'char *' drops const qualifier}}
                       ^
Sema/warn-cast-qual.c:32:19: warning: cast discards ‘const’ qualifier from pointer target type [-Wcast-qual]
   char *charptr = (char *)constcharptr; // expected-warning {{cast from 'const char *' to 'char *' drops const qualifier}}
                   ^
Sema/warn-cast-qual.c:33:31: warning: cast discards ‘const’ qualifier from pointer target type [-Wcast-qual]
   const char *constcharptr2 = (char *)constcharptr; // expected-warning {{cast from 'const char *' to 'char *' drops const qualifier}}
                               ^
Sema/warn-cast-qual.c: In function ‘bar_0’:
Sema/warn-cast-qual.c:45:4: warning: cast discards ‘const’ qualifier from pointer target type [-Wcast-qual]
   *(int *)(&S.a) = 0; // expected-warning {{cast from 'const int *' to 'int *' drops const qualifier}}
    ^
Sema/warn-cast-qual.c:46:4: warning: cast discards ‘const’ qualifier from pointer target type [-Wcast-qual]
   *(int *)(&S.b) = 0; // expected-warning {{cast from 'const int *' to 'int *' drops const qualifier}}
    ^
Sema/warn-cast-qual.c: In function ‘bar_1’:
Sema/warn-cast-qual.c:58:4: warning: cast discards ‘const’ qualifier from pointer target type [-Wcast-qual]
   *(int *)(&S.a) = 0; // expected-warning {{cast from 'const int *' to 'int *' drops const qualifier}}
    ^

One thing to note:

Sema/warn-cast-qual.c:23:31: warning: to be safe all intermediate pointers in cast from ‘int **’ to ‘volatile int **’ must be ‘const’ qualified [-Wcast-qual]
   volatile int **intptrptrv = (volatile int **)intptrptr; // expected-warning {{cast from 'int **' to 'volatile int **' must have all intermediate pointers const qualified}}
                               ^

^ both compilers talk about const qualified, even though the volatile is dropped.
2.2.

$ gcc -x c++ -fsyntax-only -Wcast-qual -c Sema/warn-cast-qual.c
Sema/warn-cast-qual.c: In function ‘void foo()’:
Sema/warn-cast-qual.c:9:21: warning: cast from type ‘const char* const’ to type ‘char*’ casts away qualifiers [-Wcast-qual]
   char *y = (char *)ptr; // expected-warning {{cast from 'const char *' to 'char *' drops const qualifier}}
                     ^~~
Sema/warn-cast-qual.c:10:24: warning: cast from type ‘const char* const*’ to type ‘char**’ casts away qualifiers [-Wcast-qual]
   char **y1 = (char **)ptrptr; // expected-warning {{cast from 'const char *const' to 'char *' drops const qualifier}}
                        ^~~~~~
Sema/warn-cast-qual.c:11:36: warning: cast from type ‘const char* const*’ to type ‘const char**’ casts away qualifiers [-Wcast-qual]
   const char **y2 = (const char **)ptrptr; // expected-warning {{cast from 'const char *const *' to 'const char **' drops const qualifier}}
                                    ^~~~~~
Sema/warn-cast-qual.c:14:36: warning: cast from type ‘const void*’ to type ‘char*’ casts away qualifiers [-Wcast-qual]
   char *z1 = (char *)(const void *)ptr; // expected-warning {{cast from 'const void *' to 'char *' drops const qualifier}}
                                    ^~~
Sema/warn-cast-qual.c:17:24: warning: cast from type ‘volatile char*’ to type ‘char*’ casts away qualifiers [-Wcast-qual]
   char *vol2 = (char *)vol; // expected-warning {{cast from 'volatile char *' to 'char *' drops volatile qualifier}}
                        ^~~
Sema/warn-cast-qual.c:19:25: warning: cast from type ‘const volatile char*’ to type ‘char*’ casts away qualifiers [-Wcast-qual]
   char *volc2 = (char *)volc; // expected-warning {{cast from 'const volatile char *' to 'char *' drops const and volatile qualifiers}}
                         ^~~~
Sema/warn-cast-qual.c:22:42: warning: cast from type ‘int**’ to type ‘const int**’ casts away qualifiers [-Wcast-qual]
   const int **intptrptrc = (const int **)intptrptr; // expected-warning {{cast from 'int **' to 'const int **' must have all intermediate pointers const qualified}}
                                          ^~~~~~~~~
Sema/warn-cast-qual.c:23:48: warning: cast from type ‘int**’ to type ‘volatile int**’ casts away qualifiers [-Wcast-qual]
   volatile int **intptrptrv = (volatile int **)intptrptr; // expected-warning {{cast from 'int **' to 'volatile int **' must have all intermediate pointers const qualified}}
                                                ^~~~~~~~~
Sema/warn-cast-qual.c:29:32: warning: cast from type ‘const char**’ to type ‘char**’ casts away qualifiers [-Wcast-qual]
   char **charptrptr = (char **)charptrptrc; // expected-warning {{cast from 'const char *' to 'char *' drops const qualifier}}
                                ^~~~~~~~~~~
Sema/warn-cast-qual.c:32:27: warning: cast from type ‘const char*’ to type ‘char*’ casts away qualifiers [-Wcast-qual]
   char *charptr = (char *)constcharptr; // expected-warning {{cast from 'const char *' to 'char *' drops const qualifier}}
                           ^~~~~~~~~~~~
Sema/warn-cast-qual.c:33:39: warning: cast from type ‘const char*’ to type ‘char*’ casts away qualifiers [-Wcast-qual]
   const char *constcharptr2 = (char *)constcharptr; // expected-warning {{cast from 'const char *' to 'char *' drops const qualifier}}
                                       ^~~~~~~~~~~~
Sema/warn-cast-qual.c: In function ‘void bar_0()’:
Sema/warn-cast-qual.c:45:16: warning: cast from type ‘const int*’ to type ‘int*’ casts away qualifiers [-Wcast-qual]
   *(int *)(&S.a) = 0; // expected-warning {{cast from 'const int *' to 'int *' drops const qualifier}}
                ^
Sema/warn-cast-qual.c:46:16: warning: cast from type ‘const int*’ to type ‘int*’ casts away qualifiers [-Wcast-qual]
   *(int *)(&S.b) = 0; // expected-warning {{cast from 'const int *' to 'int *' drops const qualifier}}
                ^
Sema/warn-cast-qual.c: In function ‘void bar_1()’:
Sema/warn-cast-qual.c:58:16: warning: cast from type ‘const int*’ to type ‘int*’ casts away qualifiers [-Wcast-qual]
   *(int *)(&S.a) = 0; // expected-warning {{cast from 'const int *' to 'int *' drops const qualifier}}
                ^

2.3.
And here are the C++ reference warnings. As you can see, gcc warnings are rather broken i'd say...

$ gcc -x c++ -fsyntax-only -Wcast-qual -c SemaCXX/warn-cast-qual.cpp
SemaCXX/warn-cast-qual.cpp: In function ‘void foo_0()’:
SemaCXX/warn-cast-qual.cpp:24:20: warning: cast from type ‘const int*’ to type ‘int*’ casts away qualifiers [-Wcast-qual]
   int &a2 = (int &)a;                      // expected-warning {{cast from 'const int' to 'int &' drops const qualifier}}
                    ^
SemaCXX/warn-cast-qual.cpp:25:26: warning: cast from type ‘const int*’ to type ‘int*’ casts away qualifiers [-Wcast-qual]
   const int &a3 = (int &)a;                // expected-warning {{cast from 'const int' to 'int &' drops const qualifier}}
                          ^
SemaCXX/warn-cast-qual.cpp:26:35: warning: cast from type ‘const int*’ to type ‘int*’ casts away qualifiers [-Wcast-qual]
   int &a4 = (int &)((const int &)a);       // expected-warning {{cast from 'const int' to 'int &' drops const qualifier}}
                                   ^
SemaCXX/warn-cast-qual.cpp:27:28: warning: cast from type ‘const int*’ to type ‘int*’ casts away qualifiers [-Wcast-qual]
   int &a5 = (int &)((int &)a);             // expected-warning {{cast from 'const int' to 'int &' drops const qualifier}}
                            ^
SemaCXX/warn-cast-qual.cpp:28:34: warning: cast from type ‘const int*’ to type ‘int*’ casts away qualifiers [-Wcast-qual]
   const int &a6 = (int &)((int &)a);       // expected-warning {{cast from 'const int' to 'int &' drops const qualifier}}
                                  ^
SemaCXX/warn-cast-qual.cpp:29:41: warning: cast from type ‘const int*’ to type ‘int*’ casts away qualifiers [-Wcast-qual]
   const int &a7 = (int &)((const int &)a); // expected-warning {{cast from 'const int' to 'int &' drops const qualifier}}
                                         ^
SemaCXX/warn-cast-qual.cpp:30:40: warning: cast from type ‘const int*’ to type ‘int*’ casts away qualifiers [-Wcast-qual]
   const int &a8 = (const int &)((int &)a); // expected-warning {{cast from 'const int' to 'int &' drops const qualifier}}
                                        ^
SemaCXX/warn-cast-qual.cpp: In function ‘void foo_1()’:
SemaCXX/warn-cast-qual.cpp:39:20: warning: cast from type ‘volatile int*’ to type ‘int*’ casts away qualifiers [-Wcast-qual]
   int &a2 = (int &)a;                            // expected-warning {{cast from 'volatile int' to 'int &' drops volatile qualifier}}
                    ^
SemaCXX/warn-cast-qual.cpp:40:29: warning: cast from type ‘volatile int*’ to type ‘int*’ casts away qualifiers [-Wcast-qual]
   volatile int &a3 = (int &)a;                   // expected-warning {{cast from 'volatile int' to 'int &' drops volatile qualifier}}
                             ^
SemaCXX/warn-cast-qual.cpp:41:38: warning: cast from type ‘volatile int*’ to type ‘int*’ casts away qualifiers [-Wcast-qual]
   int &a4 = (int &)((volatile int &)a);          // expected-warning {{cast from 'volatile int' to 'int &' drops volatile qualifier}}
                                      ^
SemaCXX/warn-cast-qual.cpp:42:28: warning: cast from type ‘volatile int*’ to type ‘int*’ casts away qualifiers [-Wcast-qual]
   int &a5 = (int &)((int &)a);                   // expected-warning {{cast from 'volatile int' to 'int &' drops volatile qualifier}}
                            ^
SemaCXX/warn-cast-qual.cpp:43:37: warning: cast from type ‘volatile int*’ to type ‘int*’ casts away qualifiers [-Wcast-qual]
   volatile int &a6 = (int &)((int &)a);          // expected-warning {{cast from 'volatile int' to 'int &' drops volatile qualifier}}
                                     ^
SemaCXX/warn-cast-qual.cpp:44:47: warning: cast from type ‘volatile int*’ to type ‘int*’ casts away qualifiers [-Wcast-qual]
   volatile int &a7 = (int &)((volatile int &)a); // expected-warning {{cast from 'volatile int' to 'int &' drops volatile qualifier}}
                                               ^
SemaCXX/warn-cast-qual.cpp:45:46: warning: cast from type ‘volatile int*’ to type ‘int*’ casts away qualifiers [-Wcast-qual]
   volatile int &a8 = (volatile int &)((int &)a); // expected-warning {{cast from 'volatile int' to 'int &' drops volatile qualifier}}
                                              ^
SemaCXX/warn-cast-qual.cpp: In function ‘void foo_2()’:
SemaCXX/warn-cast-qual.cpp:54:20: warning: cast from type ‘const volatile int*’ to type ‘int*’ casts away qualifiers [-Wcast-qual]
   int &a2 = (int &)a;                                        // expected-warning {{cast from 'const volatile int' to 'int &' drops const and volatile qualifiers}}
                    ^
SemaCXX/warn-cast-qual.cpp:55:35: warning: cast from type ‘const volatile int*’ to type ‘int*’ casts away qualifiers [-Wcast-qual]
   const volatile int &a3 = (int &)a;                         // expected-warning {{cast from 'const volatile int' to 'int &' drops const and volatile qualifiers}}
                                   ^
SemaCXX/warn-cast-qual.cpp:56:44: warning: cast from type ‘const volatile int*’ to type ‘int*’ casts away qualifiers [-Wcast-qual]
   int &a4 = (int &)((const volatile int &)a);                // expected-warning {{cast from 'const volatile int' to 'int &' drops const and volatile qualifiers}}
                                            ^
SemaCXX/warn-cast-qual.cpp:57:28: warning: cast from type ‘const volatile int*’ to type ‘int*’ casts away qualifiers [-Wcast-qual]
   int &a5 = (int &)((int &)a);                               // expected-warning {{cast from 'const volatile int' to 'int &' drops const and volatile qualifiers}}
                            ^
SemaCXX/warn-cast-qual.cpp:58:43: warning: cast from type ‘const volatile int*’ to type ‘int*’ casts away qualifiers [-Wcast-qual]
   const volatile int &a6 = (int &)((int &)a);                // expected-warning {{cast from 'const volatile int' to 'int &' drops const and volatile qualifiers}}
                                           ^
SemaCXX/warn-cast-qual.cpp:59:59: warning: cast from type ‘const volatile int*’ to type ‘int*’ casts away qualifiers [-Wcast-qual]
   const volatile int &a7 = (int &)((const volatile int &)a); // expected-warning {{cast from 'const volatile int' to 'int &' drops const and volatile qualifiers}}
                                                           ^
SemaCXX/warn-cast-qual.cpp:60:58: warning: cast from type ‘const volatile int*’ to type ‘int*’ casts away qualifiers [-Wcast-qual]
   const volatile int &a8 = (const volatile int &)((int &)a); // expected-warning {{cast from 'const volatile int' to 'int &' drops const and volatile qualifiers}}
                                                          ^
SemaCXX/warn-cast-qual.cpp: In function ‘void bar_0()’:
SemaCXX/warn-cast-qual.cpp:67:38: warning: cast from type ‘const int**’ to type ‘int**’ casts away qualifiers [-Wcast-qual]
   int **a0 = (int **)((const int **)a); // expected-warning {{cast from 'const int *' to 'int *' drops const qualifier}}
                                      ^
SemaCXX/warn-cast-qual.cpp:68:31: warning: cast from type ‘const int**’ to type ‘int**’ casts away qualifiers [-Wcast-qual]
   int **a1 = (int **)((int **)a);       // expected-warning {{cast from 'const int *' to 'int *' drops const qualifier}}
                               ^
SemaCXX/warn-cast-qual.cpp:73:43: warning: cast from type ‘const int**’ to type ‘int**’ casts away qualifiers [-Wcast-qual]
   const int **a4 = (const int **)((int **)a);        // expected-warning {{cast from 'const int *' to 'int *' drops const qualifier}} expected-warning {{cast from 'int **' to 'const int **' must have all intermediate pointers const qualified to be safe}}
                                           ^
SemaCXX/warn-cast-qual.cpp:73:44: warning: cast from type ‘int**’ to type ‘const int**’ casts away qualifiers [-Wcast-qual]
   const int **a4 = (const int **)((int **)a);        // expected-warning {{cast from 'const int *' to 'int *' drops const qualifier}} expected-warning {{cast from 'int **' to 'const int **' must have all intermediate pointers const qualified to be safe}}
                                            ^
SemaCXX/warn-cast-qual.cpp: In function ‘void bar_1()’:
SemaCXX/warn-cast-qual.cpp:81:38: warning: cast from type ‘const int**’ to type ‘int**’ casts away qualifiers [-Wcast-qual]
   int *&a0 = (int *&)((const int *&)a); // expected-warning {{cast from 'const int *' to 'int *' drops const qualifier}}
                                      ^
SemaCXX/warn-cast-qual.cpp:82:31: warning: cast from type ‘const int**’ to type ‘int**’ casts away qualifiers [-Wcast-qual]
   int *&a1 = (int *&)((int *&)a);       // expected-warning {{cast from 'const int *' to 'int *' drops const qualifier}}
                               ^
SemaCXX/warn-cast-qual.cpp:87:43: warning: cast from type ‘const int**’ to type ‘int**’ casts away qualifiers [-Wcast-qual]
   const int *&a4 = (const int *&)((int *&)a);        // expected-warning {{cast from 'const int *' to 'int *' drops const qualifier}} expected-warning {{cast from 'int *' to 'const int *&' must have all intermediate pointers const qualified to be safe}}
                                           ^
SemaCXX/warn-cast-qual.cpp:87:44: warning: cast from type ‘int**’ to type ‘const int**’ casts away qualifiers [-Wcast-qual]
   const int *&a4 = (const int *&)((int *&)a);        // expected-warning {{cast from 'const int *' to 'int *' drops const qualifier}} expected-warning {{cast from 'int *' to 'const int *&' must have all intermediate pointers const qualified to be safe}}
                                            ^
SemaCXX/warn-cast-qual.cpp: In function ‘void baz_0()’:
SemaCXX/warn-cast-qual.cpp:100:9: warning: cast from type ‘const baz_0()::C*’ to type ‘baz_0()::C*’ casts away qualifiers [-Wcast-qual]
   ((C &)S).B(); // expected-warning {{cast from 'const C' to 'C &' drops const qualifier}}
         ^
SemaCXX/warn-cast-qual.cpp:101:9: warning: cast from type ‘const baz_0()::C*’ to type ‘baz_0()::C*’ casts away qualifiers [-Wcast-qual]
   ((C &)S).A(); // expected-warning {{cast from 'const C' to 'C &' drops const qualifier}}
         ^
SemaCXX/warn-cast-qual.cpp:103:10: warning: cast from type ‘const baz_0()::C*’ to type ‘baz_0()::C*’ casts away qualifiers [-Wcast-qual]
   ((C *)&S)->B(); // expected-warning {{cast from 'const C *' to 'C *' drops const qualifier}}
          ^
SemaCXX/warn-cast-qual.cpp:104:10: warning: cast from type ‘const baz_0()::C*’ to type ‘baz_0()::C*’ casts away qualifiers [-Wcast-qual]
   ((C *)&S)->A(); // expected-warning {{cast from 'const C *' to 'C *' drops const qualifier}}
          ^
SemaCXX/warn-cast-qual.cpp: In function ‘void baz_1()’:
SemaCXX/warn-cast-qual.cpp:119:16: warning: cast from type ‘const int*’ to type ‘int*’ casts away qualifiers [-Wcast-qual]
     (int &)(S.a) = 0; // expected-warning {{cast from 'const int' to 'int &' drops const qualifier}}
                ^
SemaCXX/warn-cast-qual.cpp:122:18: warning: cast from type ‘const int*’ to type ‘int*’ casts away qualifiers [-Wcast-qual]
     *(int *)(&S.a) = 0; // expected-warning {{cast from 'const int *' to 'int *' drops const qualifier}}
                  ^
SemaCXX/warn-cast-qual.cpp:128:16: warning: cast from type ‘const int*’ to type ‘int*’ casts away qualifiers [-Wcast-qual]
     (int &)(S.a) = 0; // expected-warning {{cast from 'const int' to 'int &' drops const qualifier}}
                ^
SemaCXX/warn-cast-qual.cpp:129:16: warning: cast from type ‘const int*’ to type ‘int*’ casts away qualifiers [-Wcast-qual]
     (int &)(S.b) = 0; // expected-warning {{cast from 'const int' to 'int &' drops const qualifier}}
                ^
SemaCXX/warn-cast-qual.cpp:131:18: warning: cast from type ‘const int*’ to type ‘int*’ casts away qualifiers [-Wcast-qual]
     *(int *)(&S.a) = 0; // expected-warning {{cast from 'const int *' to 'int *' drops const qualifier}}
                  ^
SemaCXX/warn-cast-qual.cpp:132:18: warning: cast from type ‘const int*’ to type ‘int*’ casts away qualifiers [-Wcast-qual]
     *(int *)(&S.b) = 0; // expected-warning {{cast from 'const int *' to 'int *' drops const qualifier}}
                  ^

So to me it seems that for our clang's testcases, both compilers produce the compatible set of warnings...

[lebedev.ri]

(tested with:

gcc version 6.3.0 20170516 (Debian 6.3.0-18)

)

[lebedev.ri]

In D33102#773296, @dblaikie wrote:

But sure. Could you also (manually, I guess) confirm that this matches GCC's cast-qual behavior (insofar as the warning fires in the same situations). If there are any deviations, let's chat about them.

Great, you were right :)
Found a false-negative:

$ cat /tmp/tst.c
int main() {
  void* p1 = (void*)"txt";
  char* p2 = (char*)"txt";
}
$ gcc -x c -Wcast-qual /tmp/tst.c
$ gcc -x c++ -Wcast-qual /tmp/tst.c
/tmp/tst.c: In function ‘int main()’:
/tmp/tst.c:2:21: warning: cast from type ‘const char*’ to type ‘void*’ casts away qualifiers [-Wcast-qual]
   void* p1 = (void*)"txt";
                     ^~~~~
/tmp/tst.c:3:21: warning: cast from type ‘const char*’ to type ‘char*’ casts away qualifiers [-Wcast-qual]
   char* p2 = (char*)"txt";
                     ^~~~~

$ ./bin/clang -x c -Wcast-qual /tmp/tst.c 
$ ./bin/clang -x c++ -Wcast-qual /tmp/tst.c 
/tmp/tst.c:3:21: warning: cast from 'const char *' to 'char *' drops const qualifier [-Wcast-qual]
  char* p2 = (char*)"txt";
                    ^
1 warning generated.

So at least, in C++ mode, it should warn on both lines.
I'm not sure, should that really not produce a warning in C?
(gcc version 6.3.0 20170516 (Debian 6.3.0-18) )

[lebedev.ri]

After further mail exchange, i will proceed to commit this as-is.
No code changes, rebase before commit.

[lebedev.ri]

So i'm trying to analyze that stage2 warning.
The testcase seems to be: (autogenerated all the variants)

void test_nop() {
  unsigned char **ptr1 = 0;
  void **ptr2 = (void **)ptr1;
}
void test_bad() {
  unsigned char **ptr1 = 0;
  const void **ptr2 = (const void **)ptr1; // expected-warning {{cast from 'unsigned char **' to 'const void **' must have all intermediate pointers const qualified to be safe}}
}
void test_good0() {
  unsigned char **ptr1 = 0;
  void *const *ptr2 = (void *const *)ptr1;
}
void test_good1() {
  unsigned char **ptr1 = 0;
  const void *const *ptr2 = (const void *const *)ptr1;
}
void test_good2() {
  unsigned char *const *ptr1 = 0;
  void *const *ptr2 = (void *const *)ptr1;
}
void test_good3() {
  unsigned char *const *ptr1 = 0;
  const void *const *ptr2 = (const void *const *)ptr1;
}
void test_good4() {
  const unsigned char **ptr1 = 0;
  const void **ptr2 = (const void **)ptr1;
}
void test_good5() {
  const unsigned char **ptr1 = 0;
  const void *const *ptr2 = (const void *const *)ptr1;
}
void test_good6() {
  const unsigned char *const *ptr1 = 0;
  const void *const *ptr2 = (const void *const *)ptr1;
}

GCC does not warn about such code at all, clang in C mode does warn about only one combination:

$ gcc -c /tmp/test.c -Wcast-qual
$ echo $?
0
$ clang -c /tmp/test.c -Wcast-qual
/tmp/test.c:7:38: warning: cast from 'unsigned char **' to 'const void **' must have all intermediate pointers const qualified to be safe [-Wcast-qual]
  const void **ptr2 = (const void **)ptr1; // expected-warning {{cast from 'unsigned char **' to 'const void **' must have all intermediate pointers const qualified to be safe}}
                                     ^
1 warning generated.

David, you reviewed the original -Wcast-qual patch, does all that ^ make sense?

gen.cpp680 BDownload

[mclow.lists]

As of r305410, libc++ passes all the tests w/ -Wcast-qual enabled.

[lebedev.ri]

Will land this shortly..

[lebedev.ri]

Rebased before commit.

docs/ReleaseNotes.rst

 Major New Features
 ------------------
 
 -  ...
 
 Improvements to Clang's diagnostics
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
+-  -Wcast-qual was implemented for C++ (for pointers only, not references)
+
 -  -Wunused-lambda-capture warns when a variable explicitly captured
    by a lambda is not used in the body of the lambda.
 
 New Compiler Flags
 ------------------
 
 The option ....
 

lib/Sema/SemaCast.cpp

void checkNonOverloadPlaceholders() {
       SrcExpr = Self.CheckPlaceholderExpr(SrcExpr.get());
       if (SrcExpr.isInvalid())
         return;
       PlaceholderKind = (BuiltinType::Kind) 0;
     }
   };
 }
 
+static void DiagnoseCastQual(Sema &Self, const ExprResult &SrcExpr,
+                             QualType DestType);
+
 // The Try functions attempt a specific way of casting. If they succeed, they
 // return TC_Success. If their way of casting is not appropriate for the given
 // arguments, they return TC_NotApplicable and *may* set diag to a diagnostic
 // to emit if no other way succeeds. If their way of casting is appropriate but
 // fails, they return TC_Failed and *must* set diag; they can set it to 0 if
 // they emit a specialized diagnostic.
 // All diagnostics returned by these functions must expect the same three
 // arguments:
static bool UnwrapDissimilarPointerTypes(QualType& T1, QualType& T2) {
   if (T1BPType && T2BPType) {
     T1 = T1BPType->getPointeeType();
     T2 = T2BPType->getPointeeType();
     return true;
   }
   
   return false;
 }
 
 /// CastsAwayConstness - Check if the pointer conversion from SrcType to
 /// DestType casts away constness as defined in C++ 5.2.11p8ff. This is used by
 /// the cast checkers.  Both arguments must denote pointer (possibly to member)
 /// types.
 ///
 /// \param CheckCVR Whether to check for const/volatile/restrict qualifiers.
 ///
 /// \param CheckObjCLifetime Whether to check Objective-C lifetime qualifiers.
 static bool
 CastsAwayConstness(Sema &Self, QualType SrcType, QualType DestType,
                    bool CheckCVR, bool CheckObjCLifetime,
                    QualType *TheOffendingSrcType = nullptr,
                    QualType *TheOffendingDestType = nullptr,
                    Qualifiers *CastAwayQualifiers = nullptr) {
   // If the only checking we care about is for Objective-C lifetime qualifiers,
   // and we're not in ObjC mode, there's nothing to check.
   if (!CheckCVR && CheckObjCLifetime && 
       !Self.Context.getLangOpts().ObjC1)
     return false;
     
if (SrcExpr.isInvalid()) // if conversion failed, don't report another error
     return;
 
   unsigned msg = diag::err_bad_cxx_cast_generic;
   if (TryConstCast(Self, SrcExpr, DestType, /*CStyle*/false, msg) != TC_Success
       && msg != 0) {
     Self.Diag(OpRange.getBegin(), msg) << CT_Const
       << SrcExpr.get()->getType() << DestType << OpRange;
     SrcExpr = ExprError();
-  }
+  } else
+    DiagnoseCastQual(Self, SrcExpr, DestType);
 }
 
 /// Check that a reinterpret_cast\<DestType\>(SrcExpr) is not used as upcast
 /// or downcast between respective pointers or references.
 static void DiagnoseReinterpretUpDownCast(Sema &Self, const Expr *SrcExpr,
                                           QualType DestType,
                                           SourceRange OpRange) {
   QualType SrcType = SrcExpr->getType();
static TryCastResult TryReinterpretCast(Sema &Self, ExprResult &SrcExpr,
   // Void pointers are not specified, but supported by every compiler out there.
   // So we finish by allowing everything that remains - it's got to be two
   // object pointers.
   return TC_Success;
 }                                     
 
 void CastOperation::CheckCXXCStyleCast(bool FunctionalStyle,
                                        bool ListInitialization) {
+  assert(Self.getLangOpts().CPlusPlus);
+
   // Handle placeholders.
   if (isPlaceholder()) {
     // C-style casts can resolve __unknown_any types.
     if (claimPlaceholder(BuiltinType::UnknownAny)) {
       SrcExpr = Self.checkUnknownAnyCast(DestRange, DestType,
                                          SrcExpr.get(), Kind,
                                          ValueKind, BasePath);
       return;
if (tcr != TC_Success && msg != 0) {
     }
   } else if (Kind == CK_BitCast) {
     checkCastAlign();
   }
 
   // Clear out SrcExpr if there was a fatal error.
   if (tcr != TC_Success)
     SrcExpr = ExprError();
+
+  // -Wcast-qual
+  DiagnoseCastQual(Self, SrcExpr, DestType);
 }
 
 /// DiagnoseBadFunctionCast - Warn whenever a function call is cast to a 
 ///  non-matching type. Such as enum function call to int, int call to
 /// pointer; etc. Cast to 'void' is an exception.
 static void DiagnoseBadFunctionCast(Sema &Self, const ExprResult &SrcExpr,
                                   QualType DestType) {
   if (Self.Diags.isIgnored(diag::warn_bad_function_cast,
void CastOperation::CheckCStyleCast() {
   Kind = Self.PrepareScalarCast(SrcExpr, DestType);
   if (SrcExpr.isInvalid())
     return;
 
   if (Kind == CK_BitCast)
     checkCastAlign();
 
   // -Wcast-qual
+  DiagnoseCastQual(Self, SrcExpr, DestType);
+}
+
+/// DiagnoseCastQual - Warn whenever casts discards a qualifiers, be it either
+/// const, volatile or both.
+static void DiagnoseCastQual(Sema &Self, const ExprResult &SrcExpr,
+                             QualType DestType) {
+  if (SrcExpr.isInvalid())
+    return;
+
+  QualType SrcType = SrcExpr.get()->getType();
+  if (!(SrcType->isAnyPointerType() && DestType->isAnyPointerType()))
+    return;
+
   QualType TheOffendingSrcType, TheOffendingDestType;
   Qualifiers CastAwayQualifiers;
-  if (SrcType->isAnyPointerType() && DestType->isAnyPointerType() &&
-      CastsAwayConstness(Self, SrcType, DestType, true, false,
+  if (!CastsAwayConstness(Self, SrcType, DestType, true, false,
                           &TheOffendingSrcType, &TheOffendingDestType,
-                         &CastAwayQualifiers)) {
-    int qualifiers = -1;
-    if (CastAwayQualifiers.hasConst() && CastAwayQualifiers.hasVolatile()) {
-      qualifiers = 0;
-    } else if (CastAwayQualifiers.hasConst()) {
-      qualifiers = 1;
-    } else if (CastAwayQualifiers.hasVolatile()) {
-      qualifiers = 2;
-    }
-    // This is a variant of int **x; const int **y = (const int **)x;
-    if (qualifiers == -1)
-      Self.Diag(SrcExpr.get()->getLocStart(), diag::warn_cast_qual2) <<
-        SrcType << DestType;
-    else
-      Self.Diag(SrcExpr.get()->getLocStart(), diag::warn_cast_qual) <<
-        TheOffendingSrcType << TheOffendingDestType << qualifiers;
-  }
+                          &CastAwayQualifiers))
+    return;
+
+    Self.Diag(SrcExpr.get()->getLocStart(), diag::warn_cast_qual2)
+        << SrcType << DestType;
+    Self.Diag(SrcExpr.get()->getLocStart(), diag::warn_cast_qual)
+        << TheOffendingSrcType << TheOffendingDestType << qualifiers;
 }
 
 ExprResult Sema::BuildCStyleCastExpr(SourceLocation LPLoc,
                                      TypeSourceInfo *CastTypeInfo,
                                      SourceLocation RPLoc,
                                      Expr *CastExpr) {
   CastOperation Op(*this, CastTypeInfo->getType(), CastExpr);  
   Op.DestRange = CastTypeInfo->getTypeLoc().getSourceRange();

test/Sema/warn-cast-qual.c

 // RUN: %clang_cc1 -triple x86_64-unknown-unknown -fsyntax-only -Wcast-qual -verify %s
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -x c++ -fsyntax-only -Wcast-qual -verify %s
 
 #include <stdint.h>
 
 void foo() {
   const char * const ptr = 0;
   const char * const *ptrptr = 0;
   char *y = (char *)ptr;  // expected-warning {{cast from 'const char *' to 'char *' drops const qualifier}}
   char **y1 = (char **)ptrptr;  // expected-warning {{cast from 'const char *const' to 'char *' drops const qualifier}}
void foo() {
   const int **intptrptrc = (const int **)intptrptr; // expected-warning {{cast from 'int **' to 'const int **' must have all intermediate pointers const qualified}}
   volatile int **intptrptrv = (volatile int **)intptrptr; // expected-warning {{cast from 'int **' to 'volatile int **' must have all intermediate pointers const qualified}}
 
   int *intptr;
   const int *intptrc = (const int *)intptr;    // no warning
 
   const char **charptrptrc;
   char **charptrptr = (char **)charptrptrc; // expected-warning {{cast from 'const char *' to 'char *' drops const qualifier}}
+
+  const char *constcharptr;
+  char *charptr = (char *)constcharptr; // expected-warning {{cast from 'const char *' to 'char *' drops const qualifier}}
+  const char *constcharptr2 = (char *)constcharptr; // expected-warning {{cast from 'const char *' to 'char *' drops const qualifier}}
+  const char *charptr2 = (char *)charptr; // no warning
 }

test/SemaCXX/warn-cast-qual.cpp

+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -fsyntax-only -Wcast-qual -verify %s
+
+#include <stdint.h>
+
+void foo() {
+  const char * const ptr = 0;
+  const char * const *ptrptr = 0;
+  const char * const x = const_cast<const char * const>(ptr); // no warning
+  char *y = const_cast<char *>(ptr); // expected-warning {{cast from 'const char *' to 'char *' drops const qualifier}}
+  char **y1 = const_cast<char **>(ptrptr); // expected-warning {{cast from 'const char *const' to 'char *' drops const qualifier}}
+  const char **y2 = const_cast<const char **>(ptrptr); // expected-warning {{cast from 'const char *const *' to 'const char **' drops const qualifier}}
+
+  char *z = reinterpret_cast<char *>(reinterpret_cast<uintptr_t>(static_cast<const void *>(ptr))); // no warning
+  char *z1 = static_cast<char *>(const_cast<void *>(static_cast<const void *>(ptr))); // expected-warning {{cast from 'const void *' to 'void *' drops const qualifier}}
+
+  volatile char *vol = 0;
+  char *vol2 = const_cast<char *>(vol); // expected-warning {{cast from 'volatile char *' to 'char *' drops volatile qualifier}}
+  const volatile char *volc = 0;
+  char *volc2 = const_cast<char *>(volc); // expected-warning {{cast from 'const volatile char *' to 'char *' drops const and volatile qualifiers}}
+
+  int **intptrptr = 0;
+  const int **intptrptrc = const_cast<const int **>(intptrptr); // expected-warning {{cast from 'int **' to 'const int **' must have all intermediate pointers const qualified}}
+  volatile int **intptrptrv = const_cast<volatile int **>(intptrptr); // expected-warning {{cast from 'int **' to 'volatile int **' must have all intermediate pointers const qualified}}
+
+  int *intptr = 0;
+  const int *intptrc = const_cast<const int *>(intptr); // no warning
+
+  const char **charptrptrc = 0;
+  char **charptrptr = const_cast<char **>(charptrptrc); // expected-warning {{cast from 'const char *' to 'char *' drops const qualifier}}
+
+  const char *constcharptr = 0;
+  const char *constcharptr2= const_cast<const char *>(constcharptr); // no warning
+  char *charptr = const_cast<char *>(constcharptr); // expected-warning {{cast from 'const char *' to 'char *' drops const qualifier}}
+  const char *constcharptr3 = const_cast<char *>(constcharptr); // expected-warning {{cast from 'const char *' to 'char *' drops const qualifier}}
+  const char *charptr2 = static_cast<char *>(charptr); // no warning
+}
+
+// FIXME
+void bar() {
+  const int a = 0;
+
+  const int &a0 = a; // no warning
+  const int &a1 = (const int &)a; // no warning
+
+  int &a2 = (int &)a;
+  int &a3 = const_cast<int &>(a);
+
+  const int &a4 = a3;                          // no warning
+  const int &a5 = (const int &)a3;             // no warning
+  const int &a6 = const_cast<const int &>(a3); // no warning
+
+  const int &a7 = (int &)a;
+  const int &a8 = const_cast<int &>(a);
+
+  const int &a9 = a8;                           // no warning
+  const int &a10 = (const int &)a8;             // no warning
+  const int &a11 = const_cast<const int &>(a8); // no warning
+}