From 263a48b781f360fc75f4b95fb9abbb866209cb21 Mon Sep 17 00:00:00 2001 From: John McCall Date: Mon, 4 Jan 2010 23:31:57 +0000 Subject: [PATCH] Move the -Wconversion logic into SemaChecking.cpp. There's a fair amount of overlap between this and -Wsign-compare, which is why I want them in the same place. llvm-svn: 92543 --- clang/lib/Sema/Sema.cpp | 311 +------------------------------- clang/lib/Sema/Sema.h | 9 +- clang/lib/Sema/SemaChecking.cpp | 309 +++++++++++++++++++++++++++++++ 3 files changed, 315 insertions(+), 314 deletions(-) diff --git a/clang/lib/Sema/Sema.cpp b/clang/lib/Sema/Sema.cpp index 40ad90a129e7..b32ddfd2093a 100644 --- a/clang/lib/Sema/Sema.cpp +++ b/clang/lib/Sema/Sema.cpp @@ -368,315 +368,6 @@ Sema::Sema(Preprocessor &pp, ASTContext &ctxt, ASTConsumer &consumer, ExpressionEvaluationContextRecord(PotentiallyEvaluated, 0)); } -/// Retrieves the width and signedness of the given integer type, -/// or returns false if it is not an integer type. -/// -/// \param T must be canonical -static bool getIntProperties(ASTContext &C, const Type *T, - unsigned &BitWidth, bool &Signed) { - assert(T->isCanonicalUnqualified()); - - if (const VectorType *VT = dyn_cast(T)) - T = VT->getElementType().getTypePtr(); - if (const ComplexType *CT = dyn_cast(T)) - T = CT->getElementType().getTypePtr(); - - if (const BuiltinType *BT = dyn_cast(T)) { - if (!BT->isInteger()) return false; - - BitWidth = C.getIntWidth(QualType(T, 0)); - Signed = BT->isSignedInteger(); - 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 TargetWidth) { - unsigned SourceWidth = value.getBitWidth(); - 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 TargetWidth) { - if (value.isInt()) - return IsSameIntAfterCast(value.getInt(), TargetWidth); - - if (value.isVector()) { - for (unsigned i = 0, e = value.getVectorLength(); i != e; ++i) - if (!IsSameIntAfterCast(value.getVectorElt(i), TargetWidth)) - return false; - return true; - } - - if (value.isComplexInt()) { - return IsSameIntAfterCast(value.getComplexIntReal(), TargetWidth) && - IsSameIntAfterCast(value.getComplexIntImag(), TargetWidth); - } - - // This can happen with lossless casts to intptr_t of "based" lvalues. - // Assume it might use arbitrary bits. - assert(value.isLValue()); - return false; -} - - -/// 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)); -} - -/// Determines if it's reasonable for the given expression to be truncated -/// down to the given integer width. -/// * Boolean expressions are automatically white-listed. -/// * Arithmetic operations on implicitly-promoted operands of the -/// target width or less are okay --- not because the results are -/// actually guaranteed to fit within the width, but because the -/// user is effectively pretending that the operations are closed -/// within the implicitly-promoted type. -static bool IsExprValueWithinWidth(ASTContext &C, Expr *E, unsigned Width) { - E = E->IgnoreParens(); - -#ifndef NDEBUG - { - const Type *ETy = E->getType()->getCanonicalTypeInternal().getTypePtr(); - unsigned EWidth; - bool ESigned; - - if (!getIntProperties(C, ETy, EWidth, ESigned)) - assert(0 && "expression not of integer type"); - - // The caller should never let this happen. - assert(EWidth > Width && "called on expr whose type is too small"); - } -#endif - - // Strip implicit casts off. - while (isa(E)) { - E = cast(E)->getSubExpr(); - - const Type *ETy = E->getType()->getCanonicalTypeInternal().getTypePtr(); - - unsigned EWidth; - bool ESigned; - if (!getIntProperties(C, ETy, EWidth, ESigned)) - return false; - - if (EWidth <= Width) - return true; - } - - if (BinaryOperator *BO = dyn_cast(E)) { - switch (BO->getOpcode()) { - - // Boolean-valued operations are white-listed. - 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 true; - - // Operations with opaque sources are black-listed. - case BinaryOperator::PtrMemD: - case BinaryOperator::PtrMemI: - return false; - - // Left shift gets black-listed based on a judgement call. - case BinaryOperator::Shl: - return false; - - // Various special cases. - case BinaryOperator::Shr: - return IsExprValueWithinWidth(C, BO->getLHS(), Width); - case BinaryOperator::Comma: - return IsExprValueWithinWidth(C, BO->getRHS(), Width); - case BinaryOperator::Sub: - if (BO->getLHS()->getType()->isPointerType()) - return false; - // fallthrough - - // Any other operator is okay if the operands are - // promoted from expressions of appropriate size. - default: - return IsExprValueWithinWidth(C, BO->getLHS(), Width) && - IsExprValueWithinWidth(C, BO->getRHS(), Width); - } - } - - if (UnaryOperator *UO = dyn_cast(E)) { - switch (UO->getOpcode()) { - // Boolean-valued operations are white-listed. - case UnaryOperator::LNot: - return true; - - // Operations with opaque sources are black-listed. - case UnaryOperator::Deref: - case UnaryOperator::AddrOf: // should be impossible - return false; - - case UnaryOperator::OffsetOf: - return false; - - default: - return IsExprValueWithinWidth(C, UO->getSubExpr(), Width); - } - } - - // 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, C)) - if (IsSameIntAfterCast(result.Val, Width)) - return true; - - return false; -} - -/// 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.ExprEvalContexts.back().Context == 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())) - // 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) { - if (IsExprValueWithinWidth(S.Context, E, TargetWidth)) - return; - - // People want to build with -Wshorten-64-to-32 and not -Wconversion - // and by god we'll let them. - if (SourceWidth == 64 && TargetWidth == 32) - return DiagnoseImpCast(S, E, T, diag::warn_impcast_integer_64_32); - 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. @@ -697,7 +388,7 @@ void Sema::ImpCastExprToType(Expr *&Expr, QualType Ty, } } - CheckImplicitConversion(*this, Expr, Ty); + CheckImplicitConversion(Expr, Ty); if (ImplicitCastExpr *ImpCast = dyn_cast(Expr)) { if (ImpCast->getCastKind() == Kind) { diff --git a/clang/lib/Sema/Sema.h b/clang/lib/Sema/Sema.h index df25025d7737..df6f18f95b57 100644 --- a/clang/lib/Sema/Sema.h +++ b/clang/lib/Sema/Sema.h @@ -1438,10 +1438,6 @@ public: void DiagnoseSentinelCalls(NamedDecl *D, SourceLocation Loc, Expr **Args, unsigned NumArgs); - void CheckSignCompare(Expr *LHS, Expr *RHS, SourceLocation Loc, - const PartialDiagnostic &PD, - bool Equality = false); - virtual void PushExpressionEvaluationContext(ExpressionEvaluationContext NewContext); @@ -3891,6 +3887,11 @@ private: void CheckReturnStackAddr(Expr *RetValExp, QualType lhsType, SourceLocation ReturnLoc); void CheckFloatComparison(SourceLocation loc, Expr* lex, Expr* rex); + void CheckSignCompare(Expr *LHS, Expr *RHS, SourceLocation Loc, + const PartialDiagnostic &PD, + bool Equality = false); + void CheckImplicitConversion(Expr *E, QualType Target); + }; //===--------------------------------------------------------------------===// diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp index 4812a6409328..7d2e38cc1521 100644 --- a/clang/lib/Sema/SemaChecking.cpp +++ b/clang/lib/Sema/SemaChecking.cpp @@ -1578,6 +1578,219 @@ static bool IsSignBitProvablyZero(ASTContext &Context, Expr *E) { return false; } +/// Retrieves the width and signedness of the given integer type, +/// or returns false if it is not an integer type. +/// +/// \param T must be canonical +static bool getIntProperties(ASTContext &C, const Type *T, + unsigned &BitWidth, bool &Signed) { + assert(T->isCanonicalUnqualified()); + + if (const VectorType *VT = dyn_cast(T)) + T = VT->getElementType().getTypePtr(); + if (const ComplexType *CT = dyn_cast(T)) + T = CT->getElementType().getTypePtr(); + + if (const BuiltinType *BT = dyn_cast(T)) { + if (!BT->isInteger()) return false; + + BitWidth = C.getIntWidth(QualType(T, 0)); + Signed = BT->isSignedInteger(); + 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 TargetWidth) { + unsigned SourceWidth = value.getBitWidth(); + 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 TargetWidth) { + if (value.isInt()) + return IsSameIntAfterCast(value.getInt(), TargetWidth); + + if (value.isVector()) { + for (unsigned i = 0, e = value.getVectorLength(); i != e; ++i) + if (!IsSameIntAfterCast(value.getVectorElt(i), TargetWidth)) + return false; + return true; + } + + if (value.isComplexInt()) { + return IsSameIntAfterCast(value.getComplexIntReal(), TargetWidth) && + IsSameIntAfterCast(value.getComplexIntImag(), TargetWidth); + } + + // This can happen with lossless casts to intptr_t of "based" lvalues. + // Assume it might use arbitrary bits. + assert(value.isLValue()); + return false; +} + + +/// 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)); +} + +/// Determines if it's reasonable for the given expression to be truncated +/// down to the given integer width. +/// * Boolean expressions are automatically white-listed. +/// * Arithmetic operations on implicitly-promoted operands of the +/// target width or less are okay --- not because the results are +/// actually guaranteed to fit within the width, but because the +/// user is effectively pretending that the operations are closed +/// within the implicitly-promoted type. +static bool IsExprValueWithinWidth(ASTContext &C, Expr *E, unsigned Width) { + E = E->IgnoreParens(); + +#ifndef NDEBUG + { + const Type *ETy = E->getType()->getCanonicalTypeInternal().getTypePtr(); + unsigned EWidth; + bool ESigned; + + if (!getIntProperties(C, ETy, EWidth, ESigned)) + assert(0 && "expression not of integer type"); + + // The caller should never let this happen. + assert(EWidth > Width && "called on expr whose type is too small"); + } +#endif + + // Strip implicit casts off. + while (isa(E)) { + E = cast(E)->getSubExpr(); + + const Type *ETy = E->getType()->getCanonicalTypeInternal().getTypePtr(); + + unsigned EWidth; + bool ESigned; + if (!getIntProperties(C, ETy, EWidth, ESigned)) + return false; + + if (EWidth <= Width) + return true; + } + + if (BinaryOperator *BO = dyn_cast(E)) { + switch (BO->getOpcode()) { + + // Boolean-valued operations are white-listed. + 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 true; + + // Operations with opaque sources are black-listed. + case BinaryOperator::PtrMemD: + case BinaryOperator::PtrMemI: + return false; + + // Left shift gets black-listed based on a judgement call. + case BinaryOperator::Shl: + return false; + + // Various special cases. + case BinaryOperator::Shr: + return IsExprValueWithinWidth(C, BO->getLHS(), Width); + case BinaryOperator::Comma: + return IsExprValueWithinWidth(C, BO->getRHS(), Width); + case BinaryOperator::Sub: + if (BO->getLHS()->getType()->isPointerType()) + return false; + // fallthrough + + // Any other operator is okay if the operands are + // promoted from expressions of appropriate size. + default: + return IsExprValueWithinWidth(C, BO->getLHS(), Width) && + IsExprValueWithinWidth(C, BO->getRHS(), Width); + } + } + + if (UnaryOperator *UO = dyn_cast(E)) { + switch (UO->getOpcode()) { + // Boolean-valued operations are white-listed. + case UnaryOperator::LNot: + return true; + + // Operations with opaque sources are black-listed. + case UnaryOperator::Deref: + case UnaryOperator::AddrOf: // should be impossible + return false; + + case UnaryOperator::OffsetOf: + return false; + + default: + return IsExprValueWithinWidth(C, UO->getSubExpr(), Width); + } + } + + // 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, C)) + if (IsSameIntAfterCast(result.Val, Width)) + return true; + + return false; +} + /// \brief Implements -Wsign-compare. /// /// \param lex the left-hand expression @@ -1640,3 +1853,99 @@ void Sema::CheckSignCompare(Expr *lex, Expr *rex, SourceLocation OpLoc, << lex->getSourceRange() << rex->getSourceRange(); } +/// 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. +void Sema::CheckImplicitConversion(Expr *E, QualType T) { + // Don't diagnose in unevaluated contexts. + if (ExprEvalContexts.back().Context == Sema::Unevaluated) + return; + + // Don't diagnose for value-dependent expressions. + if (E->isValueDependent()) + return; + + const Type *Source = Context.getCanonicalType(E->getType()).getTypePtr(); + const Type *Target = 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(*this, 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(*this, 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, Context)) { + // Value might be a float, a float vector, or a float complex. + if (IsSameFloatAfterCast(result.Val, + Context.getFloatTypeSemantics(QualType(TargetBT, 0)), + Context.getFloatTypeSemantics(QualType(SourceBT, 0)))) + return; + } + + DiagnoseImpCast(*this, E, T, diag::warn_impcast_float_precision); + } + return; + } + + // If the target is integral, always warn. + if ((TargetBT && TargetBT->isInteger())) + // TODO: don't warn for integer values? + return DiagnoseImpCast(*this, E, T, diag::warn_impcast_float_integer); + + return; + } + + unsigned SourceWidth, TargetWidth; + bool SourceSigned, TargetSigned; + + if (!getIntProperties(Context, Source, SourceWidth, SourceSigned) || + !getIntProperties(Context, Target, TargetWidth, TargetSigned)) + return; + + if (SourceWidth > TargetWidth) { + if (IsExprValueWithinWidth(Context, E, TargetWidth)) + return; + + // People want to build with -Wshorten-64-to-32 and not -Wconversion + // and by god we'll let them. + if (SourceWidth == 64 && TargetWidth == 32) + return DiagnoseImpCast(*this, E, T, diag::warn_impcast_integer_64_32); + return DiagnoseImpCast(*this, E, T, diag::warn_impcast_integer_precision); + } + + return; +} +