From: Anton Korobeynikov Date: Fri, 14 Nov 2014 21:41:07 +0000 (+0000) Subject: Follow-up to D6217 X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=1651a71ff2b564407bf2a02819b5efcdf87382a3;p=clang Follow-up to D6217 Summary: Ok, here is somewhat addition to D6217 aiming to preserve old darwin behavior wrt the typedefed types. The actual change to SemaChecking turned out to be pretty gross, in particular: 1. We need to extract the typedef'ed type for proper diagnostics 2. We need to walk over paren expressions as well Reviewers: chandlerc, rsmith Reviewed By: rsmith Subscribers: cfe-commits Differential Revision: http://reviews.llvm.org/D6256 git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@222044 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/lib/Sema/SemaChecking.cpp b/lib/Sema/SemaChecking.cpp index 31b643f138..b27ce7c709 100644 --- a/lib/Sema/SemaChecking.cpp +++ b/lib/Sema/SemaChecking.cpp @@ -3392,6 +3392,61 @@ static bool requiresParensToAddCast(const Expr *E) { } } +static std::pair +shouldNotPrintDirectly(const ASTContext &Context, + QualType IntendedTy, + const Expr *E) { + // Use a 'while' to peel off layers of typedefs. + QualType TyTy = IntendedTy; + while (const TypedefType *UserTy = TyTy->getAs()) { + StringRef Name = UserTy->getDecl()->getName(); + QualType CastTy = llvm::StringSwitch(Name) + .Case("NSInteger", Context.LongTy) + .Case("NSUInteger", Context.UnsignedLongTy) + .Case("SInt32", Context.IntTy) + .Case("UInt32", Context.UnsignedIntTy) + .Default(QualType()); + + if (!CastTy.isNull()) + return std::make_pair(CastTy, Name); + + TyTy = UserTy->desugar(); + } + + // Strip parens if necessary. + if (const ParenExpr *PE = dyn_cast(E)) + return shouldNotPrintDirectly(Context, + PE->getSubExpr()->getType(), + PE->getSubExpr()); + + // If this is a conditional expression, then its result type is constructed + // via usual arithmetic conversions and thus there might be no necessary + // typedef sugar there. Recurse to operands to check for NSInteger & + // Co. usage condition. + if (const ConditionalOperator *CO = dyn_cast(E)) { + QualType TrueTy, FalseTy; + StringRef TrueName, FalseName; + + std::tie(TrueTy, TrueName) = + shouldNotPrintDirectly(Context, + CO->getTrueExpr()->getType(), + CO->getTrueExpr()); + std::tie(FalseTy, FalseName) = + shouldNotPrintDirectly(Context, + CO->getFalseExpr()->getType(), + CO->getFalseExpr()); + + if (TrueTy == FalseTy) + return std::make_pair(TrueTy, TrueName); + else if (TrueTy.isNull()) + return std::make_pair(FalseTy, FalseName); + else if (FalseTy.isNull()) + return std::make_pair(TrueTy, TrueName); + } + + return std::make_pair(QualType(), StringRef()); +} + bool CheckPrintfHandler::checkFormatExpr(const analyze_printf::PrintfSpecifier &FS, const char *StartSpecifier, @@ -3483,25 +3538,13 @@ CheckPrintfHandler::checkFormatExpr(const analyze_printf::PrintfSpecifier &FS, // Special-case some of Darwin's platform-independence types by suggesting // casts to primitive types that are known to be large enough. - bool ShouldNotPrintDirectly = false; + bool ShouldNotPrintDirectly = false; StringRef CastTyName; if (S.Context.getTargetInfo().getTriple().isOSDarwin()) { - // Use a 'while' to peel off layers of typedefs. - QualType TyTy = IntendedTy; - while (const TypedefType *UserTy = TyTy->getAs()) { - StringRef Name = UserTy->getDecl()->getName(); - QualType CastTy = llvm::StringSwitch(Name) - .Case("NSInteger", S.Context.LongTy) - .Case("NSUInteger", S.Context.UnsignedLongTy) - .Case("SInt32", S.Context.IntTy) - .Case("UInt32", S.Context.UnsignedIntTy) - .Default(QualType()); - - if (!CastTy.isNull()) { - ShouldNotPrintDirectly = true; - IntendedTy = CastTy; - break; - } - TyTy = UserTy->desugar(); + QualType CastTy; + std::tie(CastTy, CastTyName) = shouldNotPrintDirectly(S.Context, IntendedTy, E); + if (!CastTy.isNull()) { + IntendedTy = CastTy; + ShouldNotPrintDirectly = true; } } @@ -3518,7 +3561,7 @@ CheckPrintfHandler::checkFormatExpr(const analyze_printf::PrintfSpecifier &FS, CharSourceRange SpecRange = getSpecifierRange(StartSpecifier, SpecifierLen); - if (IntendedTy == ExprTy) { + if (IntendedTy == ExprTy && !ShouldNotPrintDirectly) { // In this case, the specifier is wrong and should be changed to match // the argument. EmitFormatDiagnostic( @@ -3572,8 +3615,11 @@ CheckPrintfHandler::checkFormatExpr(const analyze_printf::PrintfSpecifier &FS, // The expression has a type that should not be printed directly. // We extract the name from the typedef because we don't want to show // the underlying type in the diagnostic. - StringRef Name = cast(ExprTy)->getDecl()->getName(); - + StringRef Name; + if (const TypedefType *TypedefTy = dyn_cast(ExprTy)) + Name = TypedefTy->getDecl()->getName(); + else + Name = CastTyName; EmitFormatDiagnostic(S.PDiag(diag::warn_format_argument_needs_cast) << Name << IntendedTy << IsEnum << E->getSourceRange(), diff --git a/lib/Sema/SemaExpr.cpp b/lib/Sema/SemaExpr.cpp index dde3f3764f..1429df778b 100644 --- a/lib/Sema/SemaExpr.cpp +++ b/lib/Sema/SemaExpr.cpp @@ -5711,7 +5711,7 @@ QualType Sema::CheckConditionalOperands(ExprResult &Cond, ExprResult &LHS, RHS.get()->getType()->isVectorType()) return CheckVectorOperands(LHS, RHS, QuestionLoc, /*isCompAssign*/false); - UsualArithmeticConversions(LHS, RHS); + QualType ResTy = UsualArithmeticConversions(LHS, RHS); if (LHS.isInvalid() || RHS.isInvalid()) return QualType(); @@ -5728,8 +5728,12 @@ QualType Sema::CheckConditionalOperands(ExprResult &Cond, ExprResult &LHS, // If both operands have arithmetic type, do the usual arithmetic conversions // to find a common type: C99 6.5.15p3,5. - if (LHSTy->isArithmeticType() && RHSTy->isArithmeticType()) - return LHS.get()->getType(); + if (LHSTy->isArithmeticType() && RHSTy->isArithmeticType()) { + LHS = ImpCastExprToType(LHS.get(), ResTy, PrepareScalarCast(LHS, ResTy)); + RHS = ImpCastExprToType(RHS.get(), ResTy, PrepareScalarCast(RHS, ResTy)); + + return ResTy; + } // If both operands are the same structure or union type, the result is that // type. diff --git a/lib/Sema/SemaExprCXX.cpp b/lib/Sema/SemaExprCXX.cpp index 88eb41ac44..26ffbfb58e 100644 --- a/lib/Sema/SemaExprCXX.cpp +++ b/lib/Sema/SemaExprCXX.cpp @@ -4561,10 +4561,14 @@ QualType Sema::CXXCheckConditionalOperands(ExprResult &Cond, ExprResult &LHS, // the usual arithmetic conversions are performed to bring them to a // common type, and the result is of that type. if (LTy->isArithmeticType() && RTy->isArithmeticType()) { - UsualArithmeticConversions(LHS, RHS); + QualType ResTy = UsualArithmeticConversions(LHS, RHS); if (LHS.isInvalid() || RHS.isInvalid()) return QualType(); - return LHS.get()->getType(); + + LHS = ImpCastExprToType(LHS.get(), ResTy, PrepareScalarCast(LHS, ResTy)); + RHS = ImpCastExprToType(RHS.get(), ResTy, PrepareScalarCast(RHS, ResTy)); + + return ResTy; } // -- The second and third operands have pointer type, or one has pointer diff --git a/test/CodeGen/complex.c b/test/CodeGen/complex.c index 206db253ca..1c0e7cb2ee 100644 --- a/test/CodeGen/complex.c +++ b/test/CodeGen/complex.c @@ -98,3 +98,19 @@ void t8() { const _Complex double test9const = 0; _Complex double test9func() { return test9const; } + +// D6217 +void t91() { + // Check for proper type promotion of conditional expression + char c[(int)(sizeof(typeof((0 ? 2.0f : (_Complex double) 2.0f))) - sizeof(_Complex double))]; + // Check for proper codegen + (0 ? 2.0f : (_Complex double) 2.0f); +} + +void t92() { + // Check for proper type promotion of conditional expression + char c[(int)(sizeof(typeof((0 ? (_Complex double) 2.0f : 2.0f))) - sizeof(_Complex double))]; + // Check for proper codegen + (0 ? (_Complex double) 2.0f : 2.0f); +} +