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

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

2017-07-03 Thread Phabricator via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rL307045: [clang] Implement -Wcast-qual for C++ (authored by 
lebedevri).

Changed prior to commit:
  https://reviews.llvm.org/D33102?vs=105103=105108#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D33102

Files:
  cfe/trunk/docs/ReleaseNotes.rst
  cfe/trunk/lib/Sema/SemaCast.cpp
  cfe/trunk/test/Sema/warn-cast-qual.c
  cfe/trunk/test/SemaCXX/warn-cast-qual.cpp

Index: cfe/trunk/lib/Sema/SemaCast.cpp
===
--- cfe/trunk/lib/Sema/SemaCast.cpp
+++ cfe/trunk/lib/Sema/SemaCast.cpp
@@ -143,6 +143,9 @@
   };
 }
 
+static void DiagnoseCastQual(Sema , const ExprResult ,
+ 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
@@ -427,6 +430,10 @@
 /// the same kind of pointer (plain or to-member). Unlike the Sema function,
 /// this one doesn't care if the two pointers-to-member don't point into the
 /// same class. This is because CastsAwayConstness doesn't care.
+/// And additionally, it handles C++ references. If both the types are
+/// references, then their pointee types are returned,
+/// else if only one of them is reference, it's pointee type is returned,
+/// and the other type is returned as-is.
 static bool UnwrapDissimilarPointerTypes(QualType& T1, QualType& T2) {
   const PointerType *T1PtrType = T1->getAs(),
 *T2PtrType = T2->getAs();
@@ -475,6 +482,26 @@
 return true;
   }
   
+  const LValueReferenceType *T1RefType = T1->getAs(),
+*T2RefType = T2->getAs();
+  if (T1RefType && T2RefType) {
+T1 = T1RefType->getPointeeType();
+T2 = T2RefType->getPointeeType();
+return true;
+  }
+
+  if (T1RefType) {
+T1 = T1RefType->getPointeeType();
+// T2 = T2;
+return true;
+  }
+
+  if (T2RefType) {
+// T1 = T1;
+T2 = T2RefType->getPointeeType();
+return true;
+  }
+
   return false;
 }
 
@@ -503,11 +530,13 @@
   // the rules are non-trivial. So first we construct Tcv *...cv* as described
   // in C++ 5.2.11p8.
   assert((SrcType->isAnyPointerType() || SrcType->isMemberPointerType() ||
-  SrcType->isBlockPointerType()) &&
+  SrcType->isBlockPointerType() ||
+  DestType->isLValueReferenceType()) &&
  "Source type is not pointer or pointer to member.");
   assert((DestType->isAnyPointerType() || DestType->isMemberPointerType() ||
-  DestType->isBlockPointerType()) &&
- "Destination type is not pointer or pointer to member.");
+  DestType->isBlockPointerType() ||
+  DestType->isLValueReferenceType()) &&
+ "Destination type is not pointer or pointer to member, or reference.");
 
   QualType UnwrappedSrcType = Self.Context.getCanonicalType(SrcType), 
UnwrappedDestType = Self.Context.getCanonicalType(DestType);
@@ -2177,6 +2206,8 @@
 
 void CastOperation::CheckCXXCStyleCast(bool FunctionalStyle,
bool ListInitialization) {
+  assert(Self.getLangOpts().CPlusPlus);
+
   // Handle placeholders.
   if (isPlaceholder()) {
 // C-style casts can resolve __unknown_any types.
@@ -2580,30 +2611,42 @@
 
   if (Kind == CK_BitCast)
 checkCastAlign();
+}
+
+/// DiagnoseCastQual - Warn whenever casts discards a qualifiers, be it either
+/// const, volatile or both.
+static void DiagnoseCastQual(Sema , const ExprResult ,
+ QualType DestType) {
+  if (SrcExpr.isInvalid())
+return;
+
+  QualType SrcType = SrcExpr.get()->getType();
+  if (!((SrcType->isAnyPointerType() && DestType->isAnyPointerType()) ||
+DestType->isLValueReferenceType()))
+return;
 
-  // -Wcast-qual
   QualType TheOffendingSrcType, TheOffendingDestType;
   Qualifiers CastAwayQualifiers;
-  if (SrcType->isAnyPointerType() && DestType->isAnyPointerType() &&
-  CastsAwayConstness(Self, SrcType, DestType, true, false,
- , ,
- )) {
-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;
-  }
+  if (!CastsAwayConstness(Self, SrcType, DestType, true, false,
+  , ,
+

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

2017-07-03 Thread Roman Lebedev via Phabricator via cfe-commits

lebedev.ri updated this revision to Diff 105103.
lebedev.ri added a comment.

Rebased before commit.


Repository:
  rL LLVM

https://reviews.llvm.org/D33102

Files:
  docs/ReleaseNotes.rst
  lib/Sema/SemaCast.cpp
  test/Sema/warn-cast-qual.c
  test/SemaCXX/warn-cast-qual.cpp

Index: test/SemaCXX/warn-cast-qual.cpp
===
--- /dev/null
+++ test/SemaCXX/warn-cast-qual.cpp
@@ -0,0 +1,140 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -fsyntax-only -Wcast-qual -verify %s
+
+#include 
+
+// do *NOT* warn on const_cast<>()
+// use clang-tidy's cppcoreguidelines-pro-type-const-cast for that.
+void foo_ptr() {
+  const char *const ptr = 0;
+  char *t0 = const_cast(ptr); // no warning
+
+  volatile char *ptr2 = 0;
+  char *t1 = const_cast(ptr2); // no warning
+
+  const volatile char *ptr3 = 0;
+  char *t2 = const_cast(ptr3); // no warning
+}
+
+void cstr() {
+  void* p0 = (void*)(const void*)"txt"; // expected-warning {{cast from 'const void *' to 'void *' drops const qualifier}}
+  void* p1 = (void*)"txt"; // FIXME
+  char* p2 = (char*)"txt"; // expected-warning {{cast from 'const char *' to 'char *' drops const qualifier}}
+}
+
+void foo_0() {
+  const int a = 0;
+
+  const int  = a;  // no warning
+  const int  = (const int &)a; // no warning
+
+  int  = (int &)a;  // expected-warning {{cast from 'const int' to 'int &' drops const qualifier}}
+  const int  = (int &)a;// expected-warning {{cast from 'const int' to 'int &' drops const qualifier}}
+  int  = (int &)((const int &)a);   // expected-warning {{cast from 'const int' to 'int &' drops const qualifier}}
+  int  = (int &)((int &)a); // expected-warning {{cast from 'const int' to 'int &' drops const qualifier}}
+  const int  = (int &)((int &)a);   // expected-warning {{cast from 'const int' to 'int &' drops const qualifier}}
+  const int  = (int &)((const int &)a); // expected-warning {{cast from 'const int' to 'int &' drops const qualifier}}
+  const int  = (const int &)((int &)a); // expected-warning {{cast from 'const int' to 'int &' drops const qualifier}}
+}
+
+void foo_1() {
+  volatile int a = 0;
+
+  volatile int  = a; // no warning
+  volatile int  = (volatile int &)a; // no warning
+
+  int  = (int &)a;// expected-warning {{cast from 'volatile int' to 'int &' drops volatile qualifier}}
+  volatile int  = (int &)a;   // expected-warning {{cast from 'volatile int' to 'int &' drops volatile qualifier}}
+  int  = (int &)((volatile int &)a);  // expected-warning {{cast from 'volatile int' to 'int &' drops volatile qualifier}}
+  int  = (int &)((int &)a);   // expected-warning {{cast from 'volatile int' to 'int &' drops volatile qualifier}}
+  volatile int  = (int &)((int &)a);  // expected-warning {{cast from 'volatile int' to 'int &' drops volatile qualifier}}
+  volatile int  = (int &)((volatile int &)a); // expected-warning {{cast from 'volatile int' to 'int &' drops volatile qualifier}}
+  volatile int  = (volatile int &)((int &)a); // expected-warning {{cast from 'volatile int' to 'int &' drops volatile qualifier}}
+}
+
+void foo_2() {
+  const volatile int a = 0;
+
+  const volatile int  = a;   // no warning
+  const volatile int  = (const volatile int &)a; // no warning
+
+  int  = (int &)a;// expected-warning {{cast from 'const volatile int' to 'int &' drops const and volatile qualifiers}}
+  const volatile int  = (int &)a; // expected-warning {{cast from 'const volatile int' to 'int &' drops const and volatile qualifiers}}
+  int  = (int &)((const volatile int &)a);// expected-warning {{cast from 'const volatile int' to 'int &' drops const and volatile qualifiers}}
+  int  = (int &)((int &)a);   // expected-warning {{cast from 'const volatile int' to 'int &' drops const and volatile qualifiers}}
+  const volatile int  = (int &)((int &)a);// expected-warning {{cast from 'const volatile int' to 'int &' drops const and volatile qualifiers}}
+  const volatile int  = (int &)((const volatile int &)a); // expected-warning {{cast from 'const volatile int' to 'int &' drops const and volatile qualifiers}}
+  const volatile int  = (const volatile int &)((int &)a); // expected-warning {{cast from 'const volatile int' to 'int &' drops const and volatile qualifiers}}
+}
+
+void bar_0() {
+  const int *_a = 0;
+  const int **a = &_a;
+
+  int **a0 = (int **)((const int **)a); // expected-warning {{cast from 'const int *' to 'int *' drops const qualifier}}
+  int **a1 = (int **)((int **)a);   // expected-warning {{cast from 'const int *' to 'int *' drops const qualifier}}
+
+  // const int **a2 = (int **)((int **)a);
+  // const int **a3 = (int **)((const int **)a);
+
+  const int **a4 =

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

2017-07-03 Thread Roman Lebedev via Phabricator via cfe-commits

lebedev.ri added a comment.

Will land this shortly..


Repository:
  rL LLVM

https://reviews.llvm.org/D33102



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

2017-06-14 Thread Marshall Clow via Phabricator via cfe-commits

mclow.lists added a comment.

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


Repository:
  rL LLVM

https://reviews.llvm.org/D33102



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

2017-06-12 Thread David Blaikie via cfe-commits

Yeah, looks like the UB is baked in pretty deep here, so it's not
reasonable to try to fix it just because of this.

I'd probably suggest trying making that cast in PointerUnion.h into a
reinterpret cast? Would that suffice to address the const issues? Otherwise
a const_cast + reinterpret_cast?



On Mon, Jun 12, 2017 at 10:35 AM Roman Lebedev  wrote:

> On Mon, Jun 12, 2017 at 8:16 PM, David Blaikie  wrote:
> >
> >
> > On Mon, Jun 12, 2017 at 10:10 AM Roman Lebedev via Phabricator
> >  wrote:
> >>
> >> lebedev.ri added a comment.
> >>
> >> So i'm trying to analyze that stage2 warning.
> >
> >
> > Could you link to the buildbot failure to see the original LLVM project
> code
> > triggering this situation?
> http://lab.llvm.org:8011/builders/clang-with-lto-ubuntu/builds/3249
>
> http://lab.llvm.org:8011/builders/clang-with-lto-ubuntu/builds/3249/steps/build-stage2-LLVMgold.so/logs/stdio
>
> FAILED:
> /home/buildbot/Buildbot/Slave1a/clang-with-lto-ubuntu/install/stage1/bin/clang++
>   -DGTEST_HAS_RTTI=0 -DLLVM_BUILD_GLOBAL_ISEL -D_DEBUG -D_GNU_SOURCE
> -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS
> -Ilib/CodeGen/AsmPrinter
>
> -I/home/buildbot/Buildbot/Slave1a/clang-with-lto-ubuntu/llvm.src/lib/CodeGen/AsmPrinter
> -Iinclude
> -I/home/buildbot/Buildbot/Slave1a/clang-with-lto-ubuntu/llvm.src/include
> -fPIC -fvisibility-inlines-hidden -Werror -Werror=date-time -std=c++11
> -Wall -W -Wno-unused-parameter -Wwrite-strings -Wcast-qual
> -Wmissing-field-initializers -pedantic -Wno-long-long
> -Wcovered-switch-default -Wnon-virtual-dtor -Wdelete-non-virtual-dtor
> -Wstring-conversion -fcolor-diagnostics -ffunction-sections
> -fdata-sections -O3-UNDEBUG  -fno-exceptions -fno-rtti -MMD -MT
> lib/CodeGen/AsmPrinter/CMakeFiles/LLVMAsmPrinter.dir/CodeViewDebug.cpp.o
> -MF
> lib/CodeGen/AsmPrinter/CMakeFiles/LLVMAsmPrinter.dir/CodeViewDebug.cpp.o.d
> -o lib/CodeGen/AsmPrinter/CMakeFiles/LLVMAsmPrinter.dir/CodeViewDebug.cpp.o
> -c
> /home/buildbot/Buildbot/Slave1a/clang-with-lto-ubuntu/llvm.src/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp
> In file included from
>
> /home/buildbot/Buildbot/Slave1a/clang-with-lto-ubuntu/llvm.src/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp:14:
> In file included from
>
> /home/buildbot/Buildbot/Slave1a/clang-with-lto-ubuntu/llvm.src/lib/CodeGen/AsmPrinter/CodeViewDebug.h:17:
> In file included from
>
> /home/buildbot/Buildbot/Slave1a/clang-with-lto-ubuntu/llvm.src/lib/CodeGen/AsmPrinter/DbgValueHistoryCalculator.h:15:
> In file included from
>
> /home/buildbot/Buildbot/Slave1a/clang-with-lto-ubuntu/llvm.src/include/llvm/IR/DebugInfoMetadata.h:26:
> In file included from
>
> /home/buildbot/Buildbot/Slave1a/clang-with-lto-ubuntu/llvm.src/include/llvm/IR/Metadata.h:23:
>
> /home/buildbot/Buildbot/Slave1a/clang-with-lto-ubuntu/llvm.src/include/llvm/ADT/PointerUnion.h:161:19:
> error: cast from 'void **' to 'const llvm::DISubprogram **' must have
> all intermediate pointers const qualified to be safe
> [-Werror,-Wcast-qual]
> return (PT1 *)Val.getAddrOfPointer();
>   ^
>
> /home/buildbot/Buildbot/Slave1a/clang-with-lto-ubuntu/llvm.src/include/llvm/ADT/TinyPtrVector.h:177:18:
> note: in instantiation of member function 'llvm::PointerUnion llvm::DISubprogram *, llvm::SmallVector
> *>::getAddrOfPtr1' requested here
>   return Val.getAddrOfPtr1();
>  ^
>
> /home/buildbot/Buildbot/Slave1a/clang-with-lto-ubuntu/llvm.src/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp:1885:33:
> note: in instantiation of member function 'llvm::TinyPtrVector llvm::DISubprogram *>::begin' requested here
> for (const DISubprogram *SP : MethodItr.second) {
> ^
>
>
> >>
> >> 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;

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

2017-06-12 Thread Roman Lebedev via cfe-commits

On Mon, Jun 12, 2017 at 8:16 PM, David Blaikie  wrote:
>
>
> On Mon, Jun 12, 2017 at 10:10 AM Roman Lebedev via Phabricator
>  wrote:
>>
>> lebedev.ri added a comment.
>>
>> So i'm trying to analyze that stage2 warning.
>
>
> Could you link to the buildbot failure to see the original LLVM project code
> triggering this situation?
http://lab.llvm.org:8011/builders/clang-with-lto-ubuntu/builds/3249
http://lab.llvm.org:8011/builders/clang-with-lto-ubuntu/builds/3249/steps/build-stage2-LLVMgold.so/logs/stdio

FAILED: 
/home/buildbot/Buildbot/Slave1a/clang-with-lto-ubuntu/install/stage1/bin/clang++
  -DGTEST_HAS_RTTI=0 -DLLVM_BUILD_GLOBAL_ISEL -D_DEBUG -D_GNU_SOURCE
-D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS
-Ilib/CodeGen/AsmPrinter
-I/home/buildbot/Buildbot/Slave1a/clang-with-lto-ubuntu/llvm.src/lib/CodeGen/AsmPrinter
-Iinclude 
-I/home/buildbot/Buildbot/Slave1a/clang-with-lto-ubuntu/llvm.src/include
-fPIC -fvisibility-inlines-hidden -Werror -Werror=date-time -std=c++11
-Wall -W -Wno-unused-parameter -Wwrite-strings -Wcast-qual
-Wmissing-field-initializers -pedantic -Wno-long-long
-Wcovered-switch-default -Wnon-virtual-dtor -Wdelete-non-virtual-dtor
-Wstring-conversion -fcolor-diagnostics -ffunction-sections
-fdata-sections -O3-UNDEBUG  -fno-exceptions -fno-rtti -MMD -MT
lib/CodeGen/AsmPrinter/CMakeFiles/LLVMAsmPrinter.dir/CodeViewDebug.cpp.o
-MF lib/CodeGen/AsmPrinter/CMakeFiles/LLVMAsmPrinter.dir/CodeViewDebug.cpp.o.d
-o lib/CodeGen/AsmPrinter/CMakeFiles/LLVMAsmPrinter.dir/CodeViewDebug.cpp.o
-c 
/home/buildbot/Buildbot/Slave1a/clang-with-lto-ubuntu/llvm.src/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp
In file included from
/home/buildbot/Buildbot/Slave1a/clang-with-lto-ubuntu/llvm.src/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp:14:
In file included from
/home/buildbot/Buildbot/Slave1a/clang-with-lto-ubuntu/llvm.src/lib/CodeGen/AsmPrinter/CodeViewDebug.h:17:
In file included from
/home/buildbot/Buildbot/Slave1a/clang-with-lto-ubuntu/llvm.src/lib/CodeGen/AsmPrinter/DbgValueHistoryCalculator.h:15:
In file included from
/home/buildbot/Buildbot/Slave1a/clang-with-lto-ubuntu/llvm.src/include/llvm/IR/DebugInfoMetadata.h:26:
In file included from
/home/buildbot/Buildbot/Slave1a/clang-with-lto-ubuntu/llvm.src/include/llvm/IR/Metadata.h:23:
/home/buildbot/Buildbot/Slave1a/clang-with-lto-ubuntu/llvm.src/include/llvm/ADT/PointerUnion.h:161:19:
error: cast from 'void **' to 'const llvm::DISubprogram **' must have
all intermediate pointers const qualified to be safe
[-Werror,-Wcast-qual]
return (PT1 *)Val.getAddrOfPointer();
  ^
/home/buildbot/Buildbot/Slave1a/clang-with-lto-ubuntu/llvm.src/include/llvm/ADT/TinyPtrVector.h:177:18:
note: in instantiation of member function 'llvm::PointerUnion
*>::getAddrOfPtr1' requested here
  return Val.getAddrOfPtr1();
 ^
/home/buildbot/Buildbot/Slave1a/clang-with-lto-ubuntu/llvm.src/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp:1885:33:
note: in instantiation of member function 'llvm::TinyPtrVector::begin' requested here
for (const DISubprogram *SP : MethodItr.second) {
^


>>
>> 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

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

2017-06-12 Thread David Blaikie via cfe-commits

On Mon, Jun 12, 2017 at 10:10 AM Roman Lebedev via Phabricator <
revi...@reviews.llvm.org> wrote:

> lebedev.ri added a comment.
>
> So i'm trying to analyze that stage2 warning.
>

Could you link to the buildbot failure to see the original LLVM project
code triggering this situation?


> 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?
>

That seems like a reasonable warning, do you agree?

But maybe it's best to put it under another flag name so it doesn't collide
with GCC.

But really - *shrug* I'd probably leave it under the same flag, fix the
LLVM project code that causes it, and carry on.


>
> F3429854: gen.cpp 
>
>
> Repository:
>   rL LLVM
>
> https://reviews.llvm.org/D33102
>
>
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

2017-06-12 Thread Roman Lebedev via Phabricator via cfe-commits

lebedev.ri added a comment.

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?

F3429854: gen.cpp 


Repository:
  rL LLVM

https://reviews.llvm.org/D33102



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

2017-06-10 Thread Phabricator via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rL305147: [clang] Implement -Wcast-qual for C++ (authored by 
lebedevri).

Changed prior to commit:
  https://reviews.llvm.org/D33102?vs=102119=102120#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D33102

Files:
  cfe/trunk/docs/ReleaseNotes.rst
  cfe/trunk/lib/Sema/SemaCast.cpp
  cfe/trunk/test/Sema/warn-cast-qual.c
  cfe/trunk/test/SemaCXX/warn-cast-qual.cpp

Index: cfe/trunk/test/SemaCXX/warn-cast-qual.cpp
===
--- cfe/trunk/test/SemaCXX/warn-cast-qual.cpp
+++ cfe/trunk/test/SemaCXX/warn-cast-qual.cpp
@@ -0,0 +1,140 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -fsyntax-only -Wcast-qual -verify %s
+
+#include 
+
+// do *NOT* warn on const_cast<>()
+// use clang-tidy's cppcoreguidelines-pro-type-const-cast for that.
+void foo_ptr() {
+  const char *const ptr = 0;
+  char *t0 = const_cast(ptr); // no warning
+
+  volatile char *ptr2 = 0;
+  char *t1 = const_cast(ptr2); // no warning
+
+  const volatile char *ptr3 = 0;
+  char *t2 = const_cast(ptr3); // no warning
+}
+
+void cstr() {
+  void* p0 = (void*)(const void*)"txt"; // expected-warning {{cast from 'const void *' to 'void *' drops const qualifier}}
+  void* p1 = (void*)"txt"; // FIXME
+  char* p2 = (char*)"txt"; // expected-warning {{cast from 'const char *' to 'char *' drops const qualifier}}
+}
+
+void foo_0() {
+  const int a = 0;
+
+  const int  = a;  // no warning
+  const int  = (const int &)a; // no warning
+
+  int  = (int &)a;  // expected-warning {{cast from 'const int' to 'int &' drops const qualifier}}
+  const int  = (int &)a;// expected-warning {{cast from 'const int' to 'int &' drops const qualifier}}
+  int  = (int &)((const int &)a);   // expected-warning {{cast from 'const int' to 'int &' drops const qualifier}}
+  int  = (int &)((int &)a); // expected-warning {{cast from 'const int' to 'int &' drops const qualifier}}
+  const int  = (int &)((int &)a);   // expected-warning {{cast from 'const int' to 'int &' drops const qualifier}}
+  const int  = (int &)((const int &)a); // expected-warning {{cast from 'const int' to 'int &' drops const qualifier}}
+  const int  = (const int &)((int &)a); // expected-warning {{cast from 'const int' to 'int &' drops const qualifier}}
+}
+
+void foo_1() {
+  volatile int a = 0;
+
+  volatile int  = a; // no warning
+  volatile int  = (volatile int &)a; // no warning
+
+  int  = (int &)a;// expected-warning {{cast from 'volatile int' to 'int &' drops volatile qualifier}}
+  volatile int  = (int &)a;   // expected-warning {{cast from 'volatile int' to 'int &' drops volatile qualifier}}
+  int  = (int &)((volatile int &)a);  // expected-warning {{cast from 'volatile int' to 'int &' drops volatile qualifier}}
+  int  = (int &)((int &)a);   // expected-warning {{cast from 'volatile int' to 'int &' drops volatile qualifier}}
+  volatile int  = (int &)((int &)a);  // expected-warning {{cast from 'volatile int' to 'int &' drops volatile qualifier}}
+  volatile int  = (int &)((volatile int &)a); // expected-warning {{cast from 'volatile int' to 'int &' drops volatile qualifier}}
+  volatile int  = (volatile int &)((int &)a); // expected-warning {{cast from 'volatile int' to 'int &' drops volatile qualifier}}
+}
+
+void foo_2() {
+  const volatile int a = 0;
+
+  const volatile int  = a;   // no warning
+  const volatile int  = (const volatile int &)a; // no warning
+
+  int  = (int &)a;// expected-warning {{cast from 'const volatile int' to 'int &' drops const and volatile qualifiers}}
+  const volatile int  = (int &)a; // expected-warning {{cast from 'const volatile int' to 'int &' drops const and volatile qualifiers}}
+  int  = (int &)((const volatile int &)a);// expected-warning {{cast from 'const volatile int' to 'int &' drops const and volatile qualifiers}}
+  int  = (int &)((int &)a);   // expected-warning {{cast from 'const volatile int' to 'int &' drops const and volatile qualifiers}}
+  const volatile int  = (int &)((int &)a);// expected-warning {{cast from 'const volatile int' to 'int &' drops const and volatile qualifiers}}
+  const volatile int  = (int &)((const volatile int &)a); // expected-warning {{cast from 'const volatile int' to 'int &' drops const and volatile qualifiers}}
+  const volatile int  = (const volatile int &)((int &)a); // expected-warning {{cast from 'const volatile int' to 'int &' drops const and volatile qualifiers}}
+}
+
+void bar_0() {
+  const int *_a = 0;
+  const int **a = &_a;
+
+  int **a0 = (int **)((const int **)a); // expected-warning {{cast from 'const int *' to 'int *' drops const qualifier}}
+

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

2017-06-10 Thread Roman Lebedev via Phabricator via cfe-commits

lebedev.ri updated this revision to Diff 102119.
lebedev.ri edited the summary of this revision.
lebedev.ri added a comment.
This revision is now accepted and ready to land.

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


Repository:
  rL LLVM

https://reviews.llvm.org/D33102

Files:
  docs/ReleaseNotes.rst
  lib/Sema/SemaCast.cpp
  test/Sema/warn-cast-qual.c
  test/SemaCXX/warn-cast-qual.cpp

Index: test/SemaCXX/warn-cast-qual.cpp
===
--- /dev/null
+++ test/SemaCXX/warn-cast-qual.cpp
@@ -0,0 +1,140 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -fsyntax-only -Wcast-qual -verify %s
+
+#include 
+
+// do *NOT* warn on const_cast<>()
+// use clang-tidy's cppcoreguidelines-pro-type-const-cast for that.
+void foo_ptr() {
+  const char *const ptr = 0;
+  char *t0 = const_cast(ptr); // no warning
+
+  volatile char *ptr2 = 0;
+  char *t1 = const_cast(ptr2); // no warning
+
+  const volatile char *ptr3 = 0;
+  char *t2 = const_cast(ptr3); // no warning
+}
+
+void cstr() {
+  void* p0 = (void*)(const void*)"txt"; // expected-warning {{cast from 'const void *' to 'void *' drops const qualifier}}
+  void* p1 = (void*)"txt"; // FIXME
+  char* p2 = (char*)"txt"; // expected-warning {{cast from 'const char *' to 'char *' drops const qualifier}}
+}
+
+void foo_0() {
+  const int a = 0;
+
+  const int  = a;  // no warning
+  const int  = (const int &)a; // no warning
+
+  int  = (int &)a;  // expected-warning {{cast from 'const int' to 'int &' drops const qualifier}}
+  const int  = (int &)a;// expected-warning {{cast from 'const int' to 'int &' drops const qualifier}}
+  int  = (int &)((const int &)a);   // expected-warning {{cast from 'const int' to 'int &' drops const qualifier}}
+  int  = (int &)((int &)a); // expected-warning {{cast from 'const int' to 'int &' drops const qualifier}}
+  const int  = (int &)((int &)a);   // expected-warning {{cast from 'const int' to 'int &' drops const qualifier}}
+  const int  = (int &)((const int &)a); // expected-warning {{cast from 'const int' to 'int &' drops const qualifier}}
+  const int  = (const int &)((int &)a); // expected-warning {{cast from 'const int' to 'int &' drops const qualifier}}
+}
+
+void foo_1() {
+  volatile int a = 0;
+
+  volatile int  = a; // no warning
+  volatile int  = (volatile int &)a; // no warning
+
+  int  = (int &)a;// expected-warning {{cast from 'volatile int' to 'int &' drops volatile qualifier}}
+  volatile int  = (int &)a;   // expected-warning {{cast from 'volatile int' to 'int &' drops volatile qualifier}}
+  int  = (int &)((volatile int &)a);  // expected-warning {{cast from 'volatile int' to 'int &' drops volatile qualifier}}
+  int  = (int &)((int &)a);   // expected-warning {{cast from 'volatile int' to 'int &' drops volatile qualifier}}
+  volatile int  = (int &)((int &)a);  // expected-warning {{cast from 'volatile int' to 'int &' drops volatile qualifier}}
+  volatile int  = (int &)((volatile int &)a); // expected-warning {{cast from 'volatile int' to 'int &' drops volatile qualifier}}
+  volatile int  = (volatile int &)((int &)a); // expected-warning {{cast from 'volatile int' to 'int &' drops volatile qualifier}}
+}
+
+void foo_2() {
+  const volatile int a = 0;
+
+  const volatile int  = a;   // no warning
+  const volatile int  = (const volatile int &)a; // no warning
+
+  int  = (int &)a;// expected-warning {{cast from 'const volatile int' to 'int &' drops const and volatile qualifiers}}
+  const volatile int  = (int &)a; // expected-warning {{cast from 'const volatile int' to 'int &' drops const and volatile qualifiers}}
+  int  = (int &)((const volatile int &)a);// expected-warning {{cast from 'const volatile int' to 'int &' drops const and volatile qualifiers}}
+  int  = (int &)((int &)a);   // expected-warning {{cast from 'const volatile int' to 'int &' drops const and volatile qualifiers}}
+  const volatile int  = (int &)((int &)a);// expected-warning {{cast from 'const volatile int' to 'int &' drops const and volatile qualifiers}}
+  const volatile int  = (int &)((const volatile int &)a); // expected-warning {{cast from 'const volatile int' to 'int &' drops const and volatile qualifiers}}
+  const volatile int  = (const volatile int &)((int &)a); // expected-warning {{cast from 'const volatile int' to 'int &' drops const and volatile qualifiers}}
+}
+
+void bar_0() {
+  const int *_a = 0;
+  const int **a = &_a;
+
+  int **a0 = (int **)((const int **)a); // expected-warning {{cast from 'const int *' to 'int *' drops const qualifier}}
+  int **a1 = (int **)((int **)a);   // expected-warning

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

2017-06-10 Thread Roman Lebedev via cfe-commits

On Sat, Jun 10, 2017 at 7:05 PM, David Blaikie  wrote:
>
>
> On Sat, Jun 10, 2017 at 3:33 AM Roman Lebedev via Phabricator
>  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  'void *' 
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:5 main 'int (void)'
  `-CompoundStmt 
`-DeclStmt 
  `-VarDecl  col:9 p2 'void *' cinit
`-CStyleCastExpr  'void *' 
  `-ImplicitCastExpr  'void *' 
`-ImplicitCastExpr  'const char *' 
  `-StringLiteral  '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:5 main 'int ()'
  `-CompoundStmt 
`-DeclStmt 
  `-VarDecl  col:9 p2 'char *' cinit
`-CStyleCastExpr  'char *' 
  `-ImplicitCastExpr  'char *' 
`-StringLiteral  '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
>>
>>
>>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

2017-06-10 Thread David Blaikie via cfe-commits

On Sat, Jun 10, 2017 at 3:33 AM Roman Lebedev via Phabricator <
revi...@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))


> 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).

- Dave


>
>
> Repository:
>   rL LLVM
>
> https://reviews.llvm.org/D33102
>
>
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

2017-06-10 Thread Roman Lebedev via Phabricator via cfe-commits

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.
I'm not sure, should that really not produce a warning in C?
(gcc version 6.3.0 20170516 (Debian 6.3.0-18) )


Repository:
  rL LLVM

https://reviews.llvm.org/D33102



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

2017-06-09 Thread Roman Lebedev via cfe-commits

On Fri, Jun 9, 2017 at 11:28 PM, David Blaikie  wrote:
> Looks all good
OK, thank you.

> please commit whenever you're ready - if you don't have
> commit access, I (or anyone else with commit access) can commit this for
> you.
Will re-test and commit in +~12 hours.

> On Tue, Jun 6, 2017 at 1:57 PM Roman Lebedev  wrote:
>>
>> On Tue, Jun 6, 2017 at 8:52 PM, David Blaikie  wrote:
>> >
>> >
>> > On Tue, Jun 6, 2017 at 3:59 AM Roman Lebedev via Phabricator
>> >  wrote:
>> >>
>> >> lebedev.ri added a comment.
>> >>
>> >> In https://reviews.llvm.org/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:
>>
>> > Works for the positive cases, not the negative ones. (though if the
>> > counts
>> > are exactly the same, then so long as there are no false positives there
>> > aren't any false negatives either)
>> Yes, fair enough, i do not have anything to add here.
>>
>> >>
>> >>
>> >> 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]
>> >>

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

2017-06-09 Thread David Blaikie via cfe-commits

Looks all good, please commit whenever you're ready - if you don't have
commit access, I (or anyone else with commit access) can commit this for
you.

On Tue, Jun 6, 2017 at 1:57 PM Roman Lebedev  wrote:

> On Tue, Jun 6, 2017 at 8:52 PM, David Blaikie  wrote:
> >
> >
> > On Tue, Jun 6, 2017 at 3:59 AM Roman Lebedev via Phabricator
> >  wrote:
> >>
> >> lebedev.ri added a comment.
> >>
> >> In https://reviews.llvm.org/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:
>
> > Works for the positive cases, not the negative ones. (though if the
> counts
> > are exactly the same, then so long as there are no false positives there
> > aren't any false negatives either)
> Yes, fair enough, i do not have anything to add here.
>
> >>
> >>
> >> 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 **’

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

2017-06-06 Thread Roman Lebedev via cfe-commits

On Tue, Jun 6, 2017 at 8:52 PM, David Blaikie  wrote:
>
>
> On Tue, Jun 6, 2017 at 3:59 AM Roman Lebedev via Phabricator
>  wrote:
>>
>> lebedev.ri added a comment.
>>
>> In https://reviews.llvm.org/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:

> Works for the positive cases, not the negative ones. (though if the counts
> are exactly the same, then so long as there are no false positives there
> aren't any false negatives either)
Yes, fair enough, i do not have anything to add here.

>>
>>
>> 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

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

2017-06-06 Thread David Blaikie via cfe-commits

On Tue, Jun 6, 2017 at 3:59 AM Roman Lebedev via Phabricator <
revi...@reviews.llvm.org> wrote:

> lebedev.ri added a comment.
>
> In https://reviews.llvm.org/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:
>

Works for the positive cases, not the negative ones. (though if the counts
are exactly the same, then so long as there are no false positives there
aren't any false negatives either)


>
> 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

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

2017-06-06 Thread Roman Lebedev via Phabricator via cfe-commits

lebedev.ri added a comment.

(tested with:

  gcc version 6.3.0 20170516 (Debian 6.3.0-18) 
  ```)


Repository:
  rL LLVM

https://reviews.llvm.org/D33102



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

2017-06-06 Thread Roman Lebedev via Phabricator via cfe-commits

lebedev.ri added a comment.

In https://reviews.llvm.org/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}}
 ^

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

2017-06-05 Thread David Blaikie via Phabricator via cfe-commits

dblaikie accepted this revision.
dblaikie added a comment.
This revision is now accepted and ready to land.

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.


Repository:
  rL LLVM

https://reviews.llvm.org/D33102



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

2017-06-02 Thread Roman Lebedev via Phabricator via cfe-commits

lebedev.ri added a comment.

In https://reviews.llvm.org/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.

Repository:
  rL LLVM

https://reviews.llvm.org/D33102

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

2017-06-02 Thread Roman Lebedev via Phabricator via cfe-commits

lebedev.ri updated this revision to Diff 101232.
lebedev.ri marked an inline comment as done.
lebedev.ri added a subscriber: cfe-commits.
lebedev.ri added a comment.

Address review notes.


Repository:
  rL LLVM

https://reviews.llvm.org/D33102

Files:
  docs/ReleaseNotes.rst
  lib/Sema/SemaCast.cpp
  test/Sema/warn-cast-qual.c
  test/SemaCXX/warn-cast-qual.cpp

Index: test/SemaCXX/warn-cast-qual.cpp
===
--- /dev/null
+++ test/SemaCXX/warn-cast-qual.cpp
@@ -0,0 +1,134 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -fsyntax-only -Wcast-qual -verify %s
+
+#include 
+
+// do *NOT* warn on const_cast<>()
+// use clang-tidy's cppcoreguidelines-pro-type-const-cast for that.
+void foo_ptr() {
+  const char *const ptr = 0;
+  char *t0 = const_cast(ptr); // no warning
+
+  volatile char *ptr2 = 0;
+  char *t1 = const_cast(ptr2); // no warning
+
+  const volatile char *ptr3 = 0;
+  char *t2 = const_cast(ptr3); // no warning
+}
+
+void foo_0() {
+  const int a = 0;
+
+  const int  = a;  // no warning
+  const int  = (const int &)a; // no warning
+
+  int  = (int &)a;  // expected-warning {{cast from 'const int' to 'int &' drops const qualifier}}
+  const int  = (int &)a;// expected-warning {{cast from 'const int' to 'int &' drops const qualifier}}
+  int  = (int &)((const int &)a);   // expected-warning {{cast from 'const int' to 'int &' drops const qualifier}}
+  int  = (int &)((int &)a); // expected-warning {{cast from 'const int' to 'int &' drops const qualifier}}
+  const int  = (int &)((int &)a);   // expected-warning {{cast from 'const int' to 'int &' drops const qualifier}}
+  const int  = (int &)((const int &)a); // expected-warning {{cast from 'const int' to 'int &' drops const qualifier}}
+  const int  = (const int &)((int &)a); // expected-warning {{cast from 'const int' to 'int &' drops const qualifier}}
+}
+
+void foo_1() {
+  volatile int a = 0;
+
+  volatile int  = a; // no warning
+  volatile int  = (volatile int &)a; // no warning
+
+  int  = (int &)a;// expected-warning {{cast from 'volatile int' to 'int &' drops volatile qualifier}}
+  volatile int  = (int &)a;   // expected-warning {{cast from 'volatile int' to 'int &' drops volatile qualifier}}
+  int  = (int &)((volatile int &)a);  // expected-warning {{cast from 'volatile int' to 'int &' drops volatile qualifier}}
+  int  = (int &)((int &)a);   // expected-warning {{cast from 'volatile int' to 'int &' drops volatile qualifier}}
+  volatile int  = (int &)((int &)a);  // expected-warning {{cast from 'volatile int' to 'int &' drops volatile qualifier}}
+  volatile int  = (int &)((volatile int &)a); // expected-warning {{cast from 'volatile int' to 'int &' drops volatile qualifier}}
+  volatile int  = (volatile int &)((int &)a); // expected-warning {{cast from 'volatile int' to 'int &' drops volatile qualifier}}
+}
+
+void foo_2() {
+  const volatile int a = 0;
+
+  const volatile int  = a;   // no warning
+  const volatile int  = (const volatile int &)a; // no warning
+
+  int  = (int &)a;// expected-warning {{cast from 'const volatile int' to 'int &' drops const and volatile qualifiers}}
+  const volatile int  = (int &)a; // expected-warning {{cast from 'const volatile int' to 'int &' drops const and volatile qualifiers}}
+  int  = (int &)((const volatile int &)a);// expected-warning {{cast from 'const volatile int' to 'int &' drops const and volatile qualifiers}}
+  int  = (int &)((int &)a);   // expected-warning {{cast from 'const volatile int' to 'int &' drops const and volatile qualifiers}}
+  const volatile int  = (int &)((int &)a);// expected-warning {{cast from 'const volatile int' to 'int &' drops const and volatile qualifiers}}
+  const volatile int  = (int &)((const volatile int &)a); // expected-warning {{cast from 'const volatile int' to 'int &' drops const and volatile qualifiers}}
+  const volatile int  = (const volatile int &)((int &)a); // expected-warning {{cast from 'const volatile int' to 'int &' drops const and volatile qualifiers}}
+}
+
+void bar_0() {
+  const int *_a = 0;
+  const int **a = &_a;
+
+  int **a0 = (int **)((const int **)a); // expected-warning {{cast from 'const int *' to 'int *' drops const qualifier}}
+  int **a1 = (int **)((int **)a);   // expected-warning {{cast from 'const int *' to 'int *' drops const qualifier}}
+
+  // const int **a2 = (int **)((int **)a);
+  // const int **a3 = (int **)((const int **)a);
+
+  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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

22 matches

Site Navigation

Mail list logo

Footer information