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

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

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

Fixes PR4802.

Testing:
```
ninja check-clang-sema check-clang-semacxx
```

Reviewers: dblaikie, majnemer, rnk

Reviewed By: dblaikie, rnk

Subscribers: mclow.lists, cfe-commits, alexfh, rnk

Differential Revision: https://reviews.llvm.org/D33102

llvm-svn: 307045

clang/docs/ReleaseNotes.rst

@@ -52,6 +52,9 @@ Major New Features
 Improvements to Clang's diagnostics
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
+-  -Wcast-qual was implemented for C++. C-style casts are now properly
+   diagnosed.
+
 -  -Wunused-lambda-capture warns when a variable explicitly captured
    by a lambda is not used in the body of the lambda.
 

clang/lib/Sema/SemaCast.cpp

@@ -143,6 +143,9 @@ namespace {
   };
 }
 
+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
@@ -427,6 +430,10 @@ static void diagnoseBadCast(Sema &S, unsigned msg, CastType castType,
 /// 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<PointerType>(),
                     *T2PtrType = T2->getAs<PointerType>();
@@ -475,6 +482,26 @@ static bool UnwrapDissimilarPointerTypes(QualType& T1, QualType& T2) {
     return true;
   }
 
+  const LValueReferenceType *T1RefType = T1->getAs<LValueReferenceType>(),
+                            *T2RefType = T2->getAs<LValueReferenceType>();
+  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 @@ CastsAwayConstness(Sema &Self, QualType SrcType, QualType DestType,
   // 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 @@ static TryCastResult TryReinterpretCast(Sema &Self, ExprResult &SrcExpr,
 
 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 @@ void CastOperation::CheckCStyleCast() {
 
   if (Kind == CK_BitCast)
     checkCastAlign();
+}
+
+/// 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()) ||
+        DestType->isLValueReferenceType()))
+    return;
 
-  // -Wcast-qual
   QualType TheOffendingSrcType, TheOffendingDestType;
   Qualifiers CastAwayQualifiers;
-  if (SrcType->isAnyPointerType() && DestType->isAnyPointerType() &&
-      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;
-  }
+  if (!CastsAwayConstness(Self, SrcType, DestType, true, false,
+                          &TheOffendingSrcType, &TheOffendingDestType,
+                          &CastAwayQualifiers))
+    return;
+
+  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;
 }
 
 ExprResult Sema::BuildCStyleCastExpr(SourceLocation LPLoc,
@@ -2624,6 +2667,9 @@ ExprResult Sema::BuildCStyleCastExpr(SourceLocation LPLoc,
   if (Op.SrcExpr.isInvalid())
     return ExprError();
 
+  // -Wcast-qual
+  DiagnoseCastQual(Op.Self, Op.SrcExpr, Op.DestType);
+
   return Op.complete(CStyleCastExpr::Create(Context, Op.ResultType,
                               Op.ValueKind, Op.Kind, Op.SrcExpr.get(),
                               &Op.BasePath, CastTypeInfo, LPLoc, RPLoc));

clang/test/Sema/warn-cast-qual.c

@@ -1,4 +1,5 @@
 // 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>
 
@@ -26,4 +27,34 @@ void foo() {
 
   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
+}
+
+void bar_0() {
+  struct C {
+    const int a;
+    int b;
+  };
+
+  const struct C S = {0, 0};
+
+  *(int *)(&S.a) = 0; // expected-warning {{cast from 'const int *' to 'int *' drops const qualifier}}
+  *(int *)(&S.b) = 0; // expected-warning {{cast from 'const int *' to 'int *' drops const qualifier}}
+}
+
+void bar_1() {
+  struct C {
+    const int a;
+    int b;
+  };
+
+  struct C S = {0, 0};
+  S.b = 0; // no warning
+
+  *(int *)(&S.a) = 0; // expected-warning {{cast from 'const int *' to 'int *' drops const qualifier}}
+  *(int *)(&S.b) = 0; // no warning
 }

clang/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 <stdint.h>
+
+// 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<char *>(ptr); // no warning
+
+  volatile char *ptr2 = 0;
+  char *t1 = const_cast<char *>(ptr2); // no warning
+
+  const volatile char *ptr3 = 0;
+  char *t2 = const_cast<char *>(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 &a0 = a;              // no warning
+  const int &a1 = (const int &)a; // no warning
+
+  int &a2 = (int &)a;                      // expected-warning {{cast from 'const int' to 'int &' drops const qualifier}}
+  const int &a3 = (int &)a;                // expected-warning {{cast from 'const int' to 'int &' drops const qualifier}}
+  int &a4 = (int &)((const int &)a);       // expected-warning {{cast from 'const int' to 'int &' drops const qualifier}}
+  int &a5 = (int &)((int &)a);             // expected-warning {{cast from 'const int' to 'int &' drops const qualifier}}
+  const int &a6 = (int &)((int &)a);       // expected-warning {{cast from 'const int' to 'int &' drops const qualifier}}
+  const int &a7 = (int &)((const int &)a); // expected-warning {{cast from 'const int' to 'int &' drops const qualifier}}
+  const int &a8 = (const int &)((int &)a); // expected-warning {{cast from 'const int' to 'int &' drops const qualifier}}
+}
+
+void foo_1() {
+  volatile int a = 0;
+
+  volatile int &a0 = a;                 // no warning
+  volatile int &a1 = (volatile int &)a; // no warning
+
+  int &a2 = (int &)a;                            // expected-warning {{cast from 'volatile int' to 'int &' drops volatile qualifier}}
+  volatile int &a3 = (int &)a;                   // expected-warning {{cast from 'volatile int' to 'int &' drops volatile qualifier}}
+  int &a4 = (int &)((volatile int &)a);          // expected-warning {{cast from 'volatile int' to 'int &' drops volatile qualifier}}
+  int &a5 = (int &)((int &)a);                   // expected-warning {{cast from 'volatile int' to 'int &' drops volatile qualifier}}
+  volatile int &a6 = (int &)((int &)a);          // expected-warning {{cast from 'volatile int' to 'int &' drops volatile qualifier}}
+  volatile int &a7 = (int &)((volatile int &)a); // expected-warning {{cast from 'volatile int' to 'int &' drops volatile qualifier}}
+  volatile int &a8 = (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 &a0 = a;                       // no warning
+  const volatile int &a1 = (const volatile int &)a; // no warning
+
+  int &a2 = (int &)a;                                        // expected-warning {{cast from 'const volatile int' to 'int &' drops const and volatile qualifiers}}
+  const volatile int &a3 = (int &)a;                         // expected-warning {{cast from 'const volatile int' to 'int &' drops const and volatile qualifiers}}
+  int &a4 = (int &)((const volatile int &)a);                // expected-warning {{cast from 'const volatile int' to 'int &' drops const and volatile qualifiers}}
+  int &a5 = (int &)((int &)a);                               // expected-warning {{cast from 'const volatile int' to 'int &' drops const and volatile qualifiers}}
+  const volatile int &a6 = (int &)((int &)a);                // expected-warning {{cast from 'const volatile int' to 'int &' drops const and volatile qualifiers}}
+  const volatile int &a7 = (int &)((const volatile int &)a); // expected-warning {{cast from 'const volatile int' to 'int &' drops const and volatile qualifiers}}
+  const volatile int &a8 = (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 pointers const qualified to be safe}}
+  const int **a5 = (const int **)((const int **)a); // no warning
+}
+
+void bar_1() {
+  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 pointers const qualified to be safe}}
+  const int *&a5 = (const int *&)((const int *&)a); // no warning
+}
+
+void baz_0() {
+  struct C {
+    void A() {}
+    void B() const {}
+  };
+
+  const C S;
+  S.B();
+
+  ((C &)S).B(); // expected-warning {{cast from 'const C' to 'C &' drops const qualifier}}
+  ((C &)S).A(); // expected-warning {{cast from 'const C' to 'C &' drops const qualifier}}
+
+  ((C *)&S)->B(); // expected-warning {{cast from 'const C *' to 'C *' drops const qualifier}}
+  ((C *)&S)->A(); // expected-warning {{cast from 'const C *' to 'C *' drops const qualifier}}
+}
+
+void baz_1() {
+  struct C {
+    const int a;
+    int b;
+
+    C() : a(0) {}
+  };
+
+  {
+    C S;
+    S.b = 0;
+
+    (int &)(S.a) = 0; // expected-warning {{cast from 'const int' to 'int &' drops const qualifier}}
+    (int &)(S.b) = 0; // no warning
+
+    *(int *)(&S.a) = 0; // expected-warning {{cast from 'const int *' to 'int *' drops const qualifier}}
+    *(int *)(&S.b) = 0; // no warning
+  }
+  {
+    const C S;
+
+    (int &)(S.a) = 0; // expected-warning {{cast from 'const int' to 'int &' drops const qualifier}}
+    (int &)(S.b) = 0; // expected-warning {{cast from 'const int' to 'int &' drops const qualifier}}
+
+    *(int *)(&S.a) = 0; // expected-warning {{cast from 'const int *' to 'int *' drops const qualifier}}
+    *(int *)(&S.b) = 0; // expected-warning {{cast from 'const int *' to 'int *' drops const qualifier}}
+  }
+}