From 680523a91dd3351389667c8de17121ba7ae82673 Mon Sep 17 00:00:00 2001 From: John McCall Date: Sat, 7 Nov 2009 03:30:10 +0000 Subject: [PATCH] Implement -Wconversion. Off by default, in the non-gcc group. There's significant work left to be done to reduce the false-positive rate here. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@86326 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/clang/AST/Type.h | 16 ++ include/clang/Basic/DiagnosticGroups.td | 4 +- include/clang/Basic/DiagnosticSemaKinds.td | 16 ++ lib/AST/Type.cpp | 3 +- lib/Sema/Sema.cpp | 231 ++++++++++++++++++++- test/Analysis/array-struct.c | 2 +- 6 files changed, 261 insertions(+), 11 deletions(-) diff --git a/include/clang/AST/Type.h b/include/clang/AST/Type.h index daa8147391..e53b9787af 100644 --- a/include/clang/AST/Type.h +++ b/include/clang/AST/Type.h @@ -963,6 +963,22 @@ public: bool isSugared() const { return false; } QualType desugar() const { return QualType(this, 0); } + bool isInteger() const { + return TypeKind >= Bool && TypeKind <= Int128; + } + + bool isSignedInteger() const { + return TypeKind >= Char_S && TypeKind <= Int128; + } + + bool isUnsignedInteger() const { + return TypeKind >= Bool && TypeKind <= UInt128; + } + + bool isFloatingPoint() const { + return TypeKind >= Float && TypeKind <= LongDouble; + } + virtual void getAsStringInternal(std::string &InnerString, const PrintingPolicy &Policy) const; diff --git a/include/clang/Basic/DiagnosticGroups.td b/include/clang/Basic/DiagnosticGroups.td index 4146e2d5c4..76147ffed8 100644 --- a/include/clang/Basic/DiagnosticGroups.td +++ b/include/clang/Basic/DiagnosticGroups.td @@ -28,7 +28,7 @@ def : DiagGroup<"cast-align">; def : DiagGroup<"cast-qual">; def : DiagGroup<"char-align">; def Comment : DiagGroup<"comment">; -def : DiagGroup<"conversion">; +def Conversion : DiagGroup<"conversion">; def : DiagGroup<"declaration-after-statement">; def : DiagGroup<"disabled-optimization">; def : DiagGroup<"discard-qual">; @@ -162,4 +162,4 @@ def : DiagGroup<"endif-labels", [ExtraTokens]>; // endif-labels = endif-tokens // A warning group for warnings that we want to have on by default in clang, // but which aren't on by default in GCC. def NonGCC : DiagGroup<"non-gcc", - [SignCompare]>; + [SignCompare, Conversion]>; diff --git a/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td index a3cce6c163..e1cbab692e 100644 --- a/include/clang/Basic/DiagnosticSemaKinds.td +++ b/include/clang/Basic/DiagnosticSemaKinds.td @@ -632,6 +632,22 @@ def err_cconv_knr : Error< def err_cconv_varargs : Error< "variadic function cannot use '%0' calling convention">; +def warn_impcast_vector_scalar : Warning< + "implicit cast turns vector to scalar: %0 to %1">, + InGroup>, DefaultIgnore; +def warn_impcast_complex_scalar : Warning< + "implicit cast discards imaginary component: %0 to %1">, + InGroup>, DefaultIgnore; +def warn_impcast_float_precision : Warning< + "implicit cast loses floating-point precision: %0 to %1">, + InGroup>, DefaultIgnore; +def warn_impcast_float_integer : Warning< + "implicit cast turns floating-point number into integer: %0 to %1">, + InGroup>, DefaultIgnore; +def warn_impcast_integer_precision : Warning< + "implicit cast loses integer precision: %0 to %1">, + InGroup>, DefaultIgnore; + def warn_attribute_ignored_for_field_of_type : Warning< "%0 attribute ignored for field of type %1">; def warn_transparent_union_attribute_field_size_align : Warning< diff --git a/lib/AST/Type.cpp b/lib/AST/Type.cpp index 779f6808b6..e9ce830574 100644 --- a/lib/AST/Type.cpp +++ b/lib/AST/Type.cpp @@ -453,8 +453,7 @@ bool Type::isFloatingType() const { bool Type::isRealFloatingType() const { if (const BuiltinType *BT = dyn_cast(CanonicalType)) - return BT->getKind() >= BuiltinType::Float && - BT->getKind() <= BuiltinType::LongDouble; + return BT->isFloatingPoint(); if (const VectorType *VT = dyn_cast(CanonicalType)) return VT->getElementType()->isRealFloatingType(); return false; diff --git a/lib/Sema/Sema.cpp b/lib/Sema/Sema.cpp index 8104dd39d0..0da0401cb0 100644 --- a/lib/Sema/Sema.cpp +++ b/lib/Sema/Sema.cpp @@ -14,6 +14,7 @@ #include "Sema.h" #include "llvm/ADT/DenseMap.h" +#include "llvm/ADT/APFloat.h" #include "clang/AST/ASTConsumer.h" #include "clang/AST/ASTContext.h" #include "clang/AST/DeclObjC.h" @@ -364,6 +365,225 @@ Sema::Sema(Preprocessor &pp, ASTContext &ctxt, ASTConsumer &consumer, PP.getDiagnostics().SetArgToStringFn(ConvertArgToStringFn, &Context); } +/// Retrieves the width and signedness of the given integer type, +/// or returns false if it is not an integer type. +static bool getIntProperties(ASTContext &C, const Type *T, + unsigned &BitWidth, bool &Signed) { + if (const BuiltinType *BT = dyn_cast(T)) { + if (!BT->isInteger()) return false; + + BitWidth = C.getIntWidth(QualType(T, 0)); + Signed = BT->isSignedInteger(); + return true; + } + + if (const FixedWidthIntType *FWIT = dyn_cast(T)) { + BitWidth = FWIT->getWidth(); + Signed = FWIT->isSigned(); + return true; + } + + return false; +} + +/// Checks whether the given value will have the same value if it it +/// is truncated to the given width, then extended back to the +/// original width. +static bool IsSameIntAfterCast(const llvm::APSInt &value, + unsigned SourceWidth, unsigned TargetWidth) { + assert(value.getBitWidth() == SourceWidth); + llvm::APSInt truncated = value; + truncated.trunc(TargetWidth); + truncated.extend(SourceWidth); + return (truncated == value); +} + +/// Checks whether the given value will have the same value if it +/// is truncated to the given width, then extended back to the original +/// width. +/// +/// The value might be a vector or a complex. +static bool IsSameIntAfterCast(const APValue &value, unsigned Source, + unsigned Target) { + if (value.isInt()) + return IsSameIntAfterCast(value.getInt(), Source, Target); + + if (value.isVector()) { + for (unsigned i = 0, e = value.getVectorLength(); i != e; ++i) + if (!IsSameIntAfterCast(value.getVectorElt(i), Source, Target)) + return false; + return true; + } + + assert(value.isComplexInt()); + return IsSameIntAfterCast(value.getComplexIntReal(), Source, Target) && + IsSameIntAfterCast(value.getComplexIntImag(), Source, Target); +} + + +/// Checks whether the given value, which currently has the given +/// source semantics, has the same value when coerced through the +/// target semantics. +static bool IsSameFloatAfterCast(const llvm::APFloat &value, + const llvm::fltSemantics &Src, + const llvm::fltSemantics &Tgt) { + llvm::APFloat truncated = value; + + bool ignored; + truncated.convert(Src, llvm::APFloat::rmNearestTiesToEven, &ignored); + truncated.convert(Tgt, llvm::APFloat::rmNearestTiesToEven, &ignored); + + return truncated.bitwiseIsEqual(value); +} + +/// Checks whether the given value, which currently has the given +/// source semantics, has the same value when coerced through the +/// target semantics. +/// +/// The value might be a vector of floats (or a complex number). +static bool IsSameFloatAfterCast(const APValue &value, + const llvm::fltSemantics &Src, + const llvm::fltSemantics &Tgt) { + if (value.isFloat()) + return IsSameFloatAfterCast(value.getFloat(), Src, Tgt); + + if (value.isVector()) { + for (unsigned i = 0, e = value.getVectorLength(); i != e; ++i) + if (!IsSameFloatAfterCast(value.getVectorElt(i), Src, Tgt)) + return false; + return true; + } + + assert(value.isComplexFloat()); + return (IsSameFloatAfterCast(value.getComplexFloatReal(), Src, Tgt) && + IsSameFloatAfterCast(value.getComplexFloatImag(), Src, Tgt)); +} + +/// Diagnose an implicit cast; purely a helper for CheckImplicitConversion. +static void DiagnoseImpCast(Sema &S, Expr *E, QualType T, unsigned diag) { + S.Diag(E->getExprLoc(), diag) << E->getType() << T << E->getSourceRange(); +} + +/// Implements -Wconversion. +static void CheckImplicitConversion(Sema &S, Expr *E, QualType T) { + // Don't diagnose in unevaluated contexts. + if (S.ExprEvalContext == Sema::Unevaluated) + return; + + // Don't diagnose for value-dependent expressions. + if (E->isValueDependent()) + return; + + const Type *Source = S.Context.getCanonicalType(E->getType()).getTypePtr(); + const Type *Target = S.Context.getCanonicalType(T).getTypePtr(); + + // Never diagnose implicit casts to bool. + if (Target->isSpecificBuiltinType(BuiltinType::Bool)) + return; + + // Strip vector types. + if (isa(Source)) { + if (!isa(Target)) + return DiagnoseImpCast(S, E, T, diag::warn_impcast_vector_scalar); + + Source = cast(Source)->getElementType().getTypePtr(); + Target = cast(Target)->getElementType().getTypePtr(); + } + + // Strip complex types. + if (isa(Source)) { + if (!isa(Target)) + return DiagnoseImpCast(S, E, T, diag::warn_impcast_complex_scalar); + + Source = cast(Source)->getElementType().getTypePtr(); + Target = cast(Target)->getElementType().getTypePtr(); + } + + const BuiltinType *SourceBT = dyn_cast(Source); + const BuiltinType *TargetBT = dyn_cast(Target); + + // If the source is floating point... + if (SourceBT && SourceBT->isFloatingPoint()) { + // ...and the target is floating point... + if (TargetBT && TargetBT->isFloatingPoint()) { + // ...then warn if we're dropping FP rank. + + // Builtin FP kinds are ordered by increasing FP rank. + if (SourceBT->getKind() > TargetBT->getKind()) { + // Don't warn about float constants that are precisely + // representable in the target type. + Expr::EvalResult result; + if (E->Evaluate(result, S.Context)) { + // Value might be a float, a float vector, or a float complex. + if (IsSameFloatAfterCast(result.Val, + S.Context.getFloatTypeSemantics(QualType(TargetBT, 0)), + S.Context.getFloatTypeSemantics(QualType(SourceBT, 0)))) + return; + } + + DiagnoseImpCast(S, E, T, diag::warn_impcast_float_precision); + } + return; + } + + // If the target is integral, always warn. + if ((TargetBT && TargetBT->isInteger()) || + isa(Target)) + // TODO: don't warn for integer values? + return DiagnoseImpCast(S, E, T, diag::warn_impcast_float_integer); + + return; + } + + unsigned SourceWidth, TargetWidth; + bool SourceSigned, TargetSigned; + + if (!getIntProperties(S.Context, Source, SourceWidth, SourceSigned) || + !getIntProperties(S.Context, Target, TargetWidth, TargetSigned)) + return; + + if (SourceWidth > TargetWidth) { + // Don't diagnose if the expression is an integer constant + // whose value in the target type is the same as it was + // in the original type. + Expr::EvalResult result; + if (E->Evaluate(result, S.Context)) + if (IsSameIntAfterCast(result.Val, SourceWidth, TargetWidth)) + return; + + // Don't diagnose if the expression is a boolean expression. + if (Source == S.Context.IntTy.getTypePtr()) { + Expr *EIg = E->IgnoreParens(); + if (BinaryOperator *BO = dyn_cast(EIg)) { + switch (BO->getOpcode()) { + case BinaryOperator::LAnd: + case BinaryOperator::LOr: + case BinaryOperator::LT: + case BinaryOperator::GT: + case BinaryOperator::LE: + case BinaryOperator::GE: + case BinaryOperator::EQ: + case BinaryOperator::NE: + return; + default: + break; + } + } else if (UnaryOperator *UO = dyn_cast(EIg)) { + switch (UO->getOpcode()) { + case UnaryOperator::LNot: + return; + default: + break; + } + } + } + + return DiagnoseImpCast(S, E, T, diag::warn_impcast_integer_precision); + } + + return; +} + /// ImpCastExprToType - If Expr is not of type 'Type', insert an implicit cast. /// If there is already an implicit cast, merge into the existing one. /// If isLvalue, the result of the cast is an lvalue. @@ -375,18 +595,17 @@ void Sema::ImpCastExprToType(Expr *&Expr, QualType Ty, if (ExprTy == TypeTy) return; - if (Expr->getType().getTypePtr()->isPointerType() && - Ty.getTypePtr()->isPointerType()) { - QualType ExprBaseType = - cast(ExprTy.getUnqualifiedType())->getPointeeType(); - QualType BaseType = - cast(TypeTy.getUnqualifiedType())->getPointeeType(); + if (Expr->getType()->isPointerType() && Ty->isPointerType()) { + QualType ExprBaseType = cast(ExprTy)->getPointeeType(); + QualType BaseType = cast(TypeTy)->getPointeeType(); if (ExprBaseType.getAddressSpace() != BaseType.getAddressSpace()) { Diag(Expr->getExprLoc(), diag::err_implicit_pointer_address_space_cast) << Expr->getSourceRange(); } } + CheckImplicitConversion(*this, Expr, Ty); + if (ImplicitCastExpr *ImpCast = dyn_cast(Expr)) { if (ImpCast->getCastKind() == Kind) { ImpCast->setType(Ty); diff --git a/test/Analysis/array-struct.c b/test/Analysis/array-struct.c index d6b6076a14..3137967be0 100644 --- a/test/Analysis/array-struct.c +++ b/test/Analysis/array-struct.c @@ -120,7 +120,7 @@ struct s1 { // building: a->e, e->d. Only then 'a' could be added to live region roots. void f13(double timeout) { struct s1 a; - a.e.d = (long) timeout; + a.e.d = (int) timeout; if (a.e.d == 10) a.e.d = 4; } -- 2.40.0