[PATCH] D33102: [clang] Implement -Wcast-qual for C++

[Roman Lebedev via cfe-commits]

On Sat, Jun 10, 2017 at 7:05 PM, David Blaikie <dblaikie at gmail.com> wrote:
>
>
> On Sat, Jun 10, 2017 at 3:33 AM Roman Lebedev via Phabricator
> <reviews at reviews.llvm.org> wrote:
>>
>> lebedev.ri planned changes to this revision.
>> lebedev.ri added a comment.
>>
>> In https://reviews.llvm.org/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.
>
>
> Seems reasonable that it should, yes.
>
> (aside: You're still welcome to commit this patch as-is, and provide patches
> for improvements as follow-up (mostly false positives would be more of a
> concern to address before commit))
Yeah...

I have looked around, and best i can figure out is that auto-inserted
`-ImplicitCastExpr <col:21> 'void *' <BitCast>
somehow lacks the constness.
I'm not sure where it is inserted, or why the const is missing,
but i guess ImpCastExprToType function is responsible.

TranslationUnitDecl
`-FunctionDecl <line:1:1, line:3:1> line:1:5 main 'int (void)'
  `-CompoundStmt <col:12, line:3:1>
    `-DeclStmt <line:2:3, col:26>
      `-VarDecl <col:3, col:21> col:9 p2 'void *' cinit
        `-CStyleCastExpr <col:14, col:21> 'void *' <NoOp>
          `-ImplicitCastExpr <col:21> 'void *' <BitCast>
            `-ImplicitCastExpr <col:21> 'const char *' <ArrayToPointerDecay>
              `-StringLiteral <col:21> 'const char [4]' lvalue "txt"

So yeah, i will commit as-is.

>>
>> I'm not sure, should that really not produce a warning in C?
>> (gcc version 6.3.0 20170516 (Debian 6.3.0-18) )

> Probably not, no - string literals have some weird mutability in C (& in
> older versions of C++, even).
I agree. After leaving that comment, i checked AST, and in C, the type
of such string is non-const:

TranslationUnitDecl
`-FunctionDecl <line:1:1, line:3:1> line:1:5 main 'int ()'
  `-CompoundStmt <col:12, line:3:1>
    `-DeclStmt <line:2:3, col:26>
      `-VarDecl <col:3, col:21> col:9 p2 'char *' cinit
        `-CStyleCastExpr <col:14, col:21> 'char *' <NoOp>
          `-ImplicitCastExpr <col:21> 'char *' <ArrayToPointerDecay>
            `-StringLiteral <col:21> 'char [4]' lvalue "txt"


And, while there, i found out that -Wcast-align is broken for
reinterpret_cast<>() :/
https://bugs.llvm.org/show_bug.cgi?id=33397

> - Dave
Roman.

>>
>>
>>
>> Repository:
>>   rL LLVM
>>
>> https://reviews.llvm.org/D33102
>>
>>
>>
>