r222568 - Implement -Wcast-qual, fixing #13772.
Richard Smith
richard at metafoo.co.uk
Fri Nov 21 14:37:30 PST 2014
On Fri, Nov 21, 2014 at 1:03 PM, Roman Divacky <rdivacky at freebsd.org> wrote:
> Author: rdivacky
> Date: Fri Nov 21 15:03:10 2014
> New Revision: 222568
>
> URL: http://llvm.org/viewvc/llvm-project?rev=222568&view=rev
> Log:
> Implement -Wcast-qual, fixing #13772.
>
> Many thanks to dblaikie for his advices and suggestions!
>
> Added:
> cfe/trunk/test/Sema/warn-cast-qual.c
> Modified:
> cfe/trunk/include/clang/Basic/DiagnosticGroups.td
> cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
> cfe/trunk/lib/Sema/SemaCast.cpp
>
> Modified: cfe/trunk/include/clang/Basic/DiagnosticGroups.td
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticGroups.td?rev=222568&r1=222567&r2=222568&view=diff
>
> ==============================================================================
> --- cfe/trunk/include/clang/Basic/DiagnosticGroups.td (original)
> +++ cfe/trunk/include/clang/Basic/DiagnosticGroups.td Fri Nov 21 15:03:10
> 2014
> @@ -60,7 +60,7 @@ def ExternCCompat : DiagGroup<"extern-c-
> 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">;
>
> Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=222568&r1=222567&r2=222568&view=diff
>
> ==============================================================================
> --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
> +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Fri Nov 21
> 15:03:10 2014
> @@ -6115,6 +6115,11 @@ def warn_zero_size_struct_union_compat :
> 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 %select{const and "
> + "volatile qualifiers|const qualifier|volatile qualifier}2">,
> + InGroup<CastQual>, DefaultIgnore;
> +def warn_cast_qual2 : Warning<"cast from %0 to %1 must have all
> intermediate "
> + "pointers const qualified to be safe">, InGroup<CastQual>,
> DefaultIgnore;
>
I think this is overstating the point; there are safe casts that do not
have 'const' added in all intermediate points. It's a bit awkward, but we
could use something like this:
"cast from pointer to %0 to pointer to non-const type %1 is not
const-correct; storing a value through the resulting pointer may make the
original pointer point to an object that is not of type %0"
Also, can you give this diagnostic a better name, like
warn_cast_qual_indirect or warn_cast_qual_non_const_intermediate?
} // End of general sema category.
>
> // inline asm.
>
> Modified: cfe/trunk/lib/Sema/SemaCast.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaCast.cpp?rev=222568&r1=222567&r2=222568&view=diff
>
> ==============================================================================
> --- cfe/trunk/lib/Sema/SemaCast.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaCast.cpp Fri Nov 21 15:03:10 2014
> @@ -142,9 +142,6 @@ namespace {
> };
> }
>
> -static bool CastsAwayConstness(Sema &Self, QualType SrcType, QualType
> DestType,
> - bool CheckCVR, bool CheckObjCLifetime);
> -
> // 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
> @@ -462,7 +459,10 @@ static bool UnwrapDissimilarPointerTypes
> /// \param CheckObjCLifetime Whether to check Objective-C lifetime
> qualifiers.
> static bool
> CastsAwayConstness(Sema &Self, QualType SrcType, QualType DestType,
> - bool CheckCVR, bool CheckObjCLifetime) {
> + bool CheckCVR, bool CheckObjCLifetime,
> + QualType *TheOffendingSrcType = nullptr,
> + QualType *TheOffendingDestType = nullptr,
> + Qualifiers *CastAwayQualifiers = nullptr) {
> // If the only checking we care about is for Objective-C lifetime
> qualifiers,
> // and we're not in ARC mode, there's nothing to check.
> if (!CheckCVR && CheckObjCLifetime &&
> @@ -487,6 +487,8 @@ CastsAwayConstness(Sema &Self, QualType
> // Find the qualifiers. We only care about cvr-qualifiers for the
> // purpose of this check, because other qualifiers (address spaces,
> // Objective-C GC, etc.) are part of the type's identity.
> + QualType PrevUnwrappedSrcType = UnwrappedSrcType;
> + QualType PrevUnwrappedDestType = UnwrappedDestType;
> while (UnwrapDissimilarPointerTypes(UnwrappedSrcType,
> UnwrappedDestType)) {
> // Determine the relevant qualifiers at this level.
> Qualifiers SrcQuals, DestQuals;
> @@ -497,6 +499,13 @@ CastsAwayConstness(Sema &Self, QualType
> if (CheckCVR) {
> RetainedSrcQuals.setCVRQualifiers(SrcQuals.getCVRQualifiers());
> RetainedDestQuals.setCVRQualifiers(DestQuals.getCVRQualifiers());
> +
> + if (RetainedSrcQuals != RetainedDestQuals && TheOffendingSrcType &&
> + TheOffendingDestType && CastAwayQualifiers) {
> + *TheOffendingSrcType = PrevUnwrappedSrcType;
> + *TheOffendingDestType = PrevUnwrappedDestType;
> + *CastAwayQualifiers = RetainedSrcQuals - RetainedDestQuals;
> + }
> }
>
> if (CheckObjCLifetime &&
> @@ -505,6 +514,9 @@ CastsAwayConstness(Sema &Self, QualType
>
> cv1.push_back(RetainedSrcQuals);
> cv2.push_back(RetainedDestQuals);
> +
> + PrevUnwrappedSrcType = UnwrappedSrcType;
> + PrevUnwrappedDestType = UnwrappedDestType;
> }
> if (cv1.empty())
> return false;
> @@ -2371,6 +2383,30 @@ void CastOperation::CheckCStyleCast() {
>
> if (Kind == CK_BitCast)
> checkCastAlign();
> +
> + // -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;
> + }
> }
>
> ExprResult Sema::BuildCStyleCastExpr(SourceLocation LPLoc,
>
> Added: cfe/trunk/test/Sema/warn-cast-qual.c
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/warn-cast-qual.c?rev=222568&view=auto
>
> ==============================================================================
> --- cfe/trunk/test/Sema/warn-cast-qual.c (added)
> +++ cfe/trunk/test/Sema/warn-cast-qual.c Fri Nov 21 15:03:10 2014
> @@ -0,0 +1,29 @@
> +// 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}}
>
I don't think this wording is precise enough -- we are not casting from
'const char *const' to 'char*', we are casting from 'const char *const *'
to 'char**'. If you want to strip off layers of pointers here, you should
indicate you did so in the diagnostic wording: "cast from pointer to 'const
char * const' to pointer to 'char *' drops const qualifier" would be better.
+ 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 and volatile qualifiers}}
> +
> + int **intptrptr;
> + const int **intptrptrc = (const int **)intptrptr; // expected-warning
> {{cast from 'int **' to 'const int **' must have all intermediate pointers
> const qualified}}
> + volatile int **intptrptrv = (volatile int **)intptrptr; //
> expected-warning {{cast from 'int **' to 'volatile int **' must have all
> intermediate pointers const qualified}}
> +
> + int *intptr;
> + const int *intptrc = (const int *)intptr; // no warning
> +
> + const char **charptrptrc;
> + char **charptrptr = (char **)charptrptrc; // expected-warning {{cast
> from 'const char *' to 'char *' drops const qualifier}}
> +}
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20141121/dc53fc35/attachment.html>
More information about the cfe-commits
mailing list