Like this?
On Fri, Nov 14, 2014 at 11:20:49AM -0800, David Blaikie wrote:
> I take it this is consistent with the GCC warning - in terms of warning on
> the innermost issue, reporting const or volatile - what about dropping
> const and volatile at the same time?
>
> Issues with the current code:
>
> * DestPtr and SrcPtr don't need to be initialized to null, they'll be
> written to on the first loop iteration as needed - avoiding excess
> initialization helps tools like MSan find more bugs rather than the program
> silently using unintended default values
>
> * InnerMostDestType and InnerMostSrcType will be dangling pointers after
> the while loop (so accessing them in the proceeding 'if' is UB)
>
> * you don't need to check both InnerMostDestType and InnerMostSrcType in
> the following if - it's invariant that if one is non-null (you can use
> QualType values rather than QualType* to address the previous bug, and use
> QualTypes "isNull()" member function here) so is the other
>
> On Fri, Nov 14, 2014 at 11:07 AM, Roman Divacky <[email protected]> wrote:
>
> > Actually, try this patch. It includes check for volatile as well.
> >
> > On Wed, Nov 12, 2014 at 12:39:20PM -0800, David Blaikie wrote:
> > > [+Richard for oversight]
> > >
> > > char **y1 = (char **)ptrptr; // expected-warning {{cast from 'const char
> > > *const *' to 'char **' drops const qualifier}} expected-warning {{cast
> > from
> > > 'const char *const' to 'char *' drops const qualifier}}
> > >
> > > I think if we're going to warn on multiple layers (I'm not sure that's
> > > ideal - is that consistent with GCC's warning? Does GCC warn on
> > mismatched
> > > types too - "const T*" -> "U*"? - do we warn there too, or only when
> > > there's a valid implicit conversion like the void* example?) then we
> > should
> > > probably drop the top level const, "const char *const" -> "char*" - the
> > top
> > > level const on the first type is confusing/misleading, it's only relevant
> > > to show "const char*" and "char*".
> > >
> > >
> > > On Wed, Nov 12, 2014 at 10:41 AM, Roman Divacky <[email protected]>
> > wrote:
> > >
> > > > I expanded the testcase and fixed the grammar in the actual warning.
> > > >
> > > > New patch attached.
> > > >
> > > > On Tue, Nov 11, 2014 at 05:03:33PM -0800, David Blaikie wrote:
> > > > > (it's a bit easier if you include the test in the same patch file -
> > also
> > > > > you can use Phabricator if you like - some reviewers perefer it)
> > > > >
> > > > > Since you've got the loop there for seeing through multiple levels of
> > > > > pointer, should you have a test case that exercises that on a > 1
> > level
> > > > of
> > > > > depth? Demonstrate that we warn on both levels (if that's the right
> > thing
> > > > > to do?)?
> > > > >
> > > > > Optionally (probably in a separate follow-up patch) you could add a
> > note
> > > > > with a fixit to include the missing consts.
> > > > >
> > > > > On Tue, Nov 11, 2014 at 10:58 AM, Roman Divacky <[email protected]>
> > > > wrote:
> > > > >
> > > > > > Hi,
> > > > > >
> > > > > > I implemented -Wcast-qual. The patch is actually quite short
> > (attached
> > > > + a
> > > > > > test
> > > > > > case).
> > > > > >
> > > > > > This fixes #13772 and also note that -Wcast-qual is used in llvm
> > build
> > > > > > itself.
> > > > > >
> > > > > > Is this ok to be commited? Thanks
> > > > > >
> > > > > > Roman
> > > > > >
> > > > > >
> > > > > > _______________________________________________
> > > > > > cfe-commits mailing list
> > > > > > [email protected]
> > > > > > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
> > > > > >
> > > > > >
> > > >
> >
Index: include/clang/Basic/DiagnosticGroups.td
===================================================================
--- include/clang/Basic/DiagnosticGroups.td (revision 221980)
+++ include/clang/Basic/DiagnosticGroups.td (working copy)
@@ -60,7 +60,7 @@
def KeywordCompat : DiagGroup<"keyword-compat">;
def GNUCaseRange : DiagGroup<"gnu-case-range">;
def CastAlign : DiagGroup<"cast-align">;
-def : DiagGroup<"cast-qual">;
+def CastQual : DiagGroup<"cast-qual">;
def : DiagGroup<"char-align">;
def Comment : DiagGroup<"comment">;
def GNUComplexInteger : DiagGroup<"gnu-complex-integer">;
Index: include/clang/Basic/DiagnosticSemaKinds.td
===================================================================
--- include/clang/Basic/DiagnosticSemaKinds.td (revision 221980)
+++ include/clang/Basic/DiagnosticSemaKinds.td (working copy)
@@ -6099,6 +6099,8 @@
def warn_zero_size_struct_union_in_extern_c : Warning<"%select{|empty }0"
"%select{struct|union}1 has size 0 in C, %select{size 1|non-zero size}2 in C++">,
InGroup<ExternCCompat>;
+def warn_cast_qual : Warning<"cast from %0 to %1 drops %2 qualifier">,
+ InGroup<CastQual>, DefaultIgnore;
} // End of general sema category.
// inline asm.
Index: lib/Sema/SemaCast.cpp
===================================================================
--- lib/Sema/SemaCast.cpp (revision 221980)
+++ lib/Sema/SemaCast.cpp (working copy)
@@ -2371,6 +2371,30 @@
if (Kind == CK_BitCast)
checkCastAlign();
+
+ // -Wcast-qual
+ const PointerType *DestPtr;
+ const PointerType *SrcPtr;
+ QualType InnerMostDestType;
+ QualType InnerMostSrcType;
+ const char *constQualDropped = nullptr;
+
+ while ((DestPtr = DestType->getAs<PointerType>()) &&
+ (SrcPtr = SrcType->getAs<PointerType>())) {
+ QualType OrigDestType = DestType;
+ QualType OrigSrcType = SrcType;
+ DestType = DestPtr->getPointeeType();
+ SrcType = SrcPtr->getPointeeType();
+ if ((SrcType.isConstQualified() && !DestType.isConstQualified()) ||
+ (SrcType.isVolatileQualified() && !DestType.isVolatileQualified())) {
+ InnerMostDestType = OrigDestType;
+ InnerMostSrcType = OrigSrcType;
+ constQualDropped = SrcType.getQualifiers().getAsString().c_str();
+ }
+ }
+ if (!InnerMostDestType.isNull())
+ Self.Diag(SrcExpr.get()->getLocStart(), diag::warn_cast_qual) <<
+ InnerMostSrcType << InnerMostDestType << constQualDropped;
}
ExprResult Sema::BuildCStyleCastExpr(SourceLocation LPLoc,
Index: test/Sema/warn-cast-qual.c
===================================================================
--- test/Sema/warn-cast-qual.c (revision 0)
+++ test/Sema/warn-cast-qual.c (working copy)
@@ -0,0 +1,19 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -fsyntax-only -Wcast-qual -verify %s
+
+#include <stdint.h>
+
+void foo() {
+ const char * const ptr = 0;
+ const char * const *ptrptr = 0;
+ char *y = (char *)ptr; // expected-warning {{cast from 'const char *' to 'char *' drops const qualifier}}
+ char **y1 = (char **)ptrptr; // expected-warning {{cast from 'const char *const' to 'char *' drops const qualifier}}
+ const char **y2 = (const char **)ptrptr; // expected-warning {{cast from 'const char *const' to 'const char *' drops const qualifier}}
+
+ char *z = (char *)(uintptr_t)(const void *)ptr; // no warning
+ char *z1 = (char *)(const void *)ptr; // expected-warning {{cast from 'const void *' to 'char *' drops const qualifier}}
+
+ volatile char *vol = 0;
+ char *vol2 = (char *)vol; // expected-warning {{cast from 'volatile char *' to 'char *' drops volatile qualifier}}
+ const volatile char *volc = 0;
+ char *volc2 = (char *)volc; // expected-warning {{cast from 'const volatile char *' to 'char *' drops const volatile qualifier}}
+}
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
