From 8b7b1b1aee8685c34130f7e4a8457a52558af034 Mon Sep 17 00:00:00 2001 From: Eli Friedman Date: Sat, 28 Mar 2009 01:22:36 +0000 Subject: [PATCH] Change compound assignment operators to keep track of both the promoted LHS type and the computation result type; this encodes information into the AST which is otherwise non-obvious. Fix Sema to always come up with the right answer for both of these types. Fix IRGen and the analyzer to account for these changes. This fixes PR2601. The approach is inspired by PR2601 comment 2. Note that this changes real *= complex in CodeGen from a silent miscompilation to an explicit error. I'm not really sure that the analyzer changes are correct, or how to test them... someone more familiar with the analyzer should check those changes. llvm-svn: 67889 --- clang/include/clang/AST/Expr.h | 15 +++- clang/lib/AST/StmtDumper.cpp | 6 +- clang/lib/AST/StmtSerialization.cpp | 8 +- clang/lib/Analysis/GRExprEngine.cpp | 16 ++-- clang/lib/CodeGen/CGExprComplex.cpp | 2 +- clang/lib/CodeGen/CGExprScalar.cpp | 79 +++++-------------- clang/lib/Sema/Sema.h | 4 +- clang/lib/Sema/SemaExpr.cpp | 113 ++++++++++++++++++---------- clang/test/CodeGen/complex.c | 3 +- clang/test/CodeGen/compound-type.c | 7 ++ 10 files changed, 130 insertions(+), 123 deletions(-) create mode 100644 clang/test/CodeGen/compound-type.c diff --git a/clang/include/clang/AST/Expr.h b/clang/include/clang/AST/Expr.h index f652f1d26e97..2271a4e6bb34 100644 --- a/clang/include/clang/AST/Expr.h +++ b/clang/include/clang/AST/Expr.h @@ -1324,18 +1324,25 @@ protected: /// place. This captures the intermediate type which the computation is done /// in. class CompoundAssignOperator : public BinaryOperator { - QualType ComputationType; + QualType ComputationLHSType; + QualType ComputationResultType; public: CompoundAssignOperator(Expr *lhs, Expr *rhs, Opcode opc, - QualType ResType, QualType CompType, + QualType ResType, QualType CompLHSType, + QualType CompResultType, SourceLocation OpLoc) : BinaryOperator(lhs, rhs, opc, ResType, OpLoc, true), - ComputationType(CompType) { + ComputationLHSType(CompLHSType), + ComputationResultType(CompResultType) { assert(isCompoundAssignmentOp() && "Only should be used for compound assignments"); } - QualType getComputationType() const { return ComputationType; } + // The two computation types are the type the LHS is converted + // to for the computation and the type of the result; the two are + // distinct in a few cases (specifically, int+=ptr and ptr-=ptr). + QualType getComputationLHSType() const { return ComputationLHSType; } + QualType getComputationResultType() const { return ComputationResultType; } static bool classof(const CompoundAssignOperator *) { return true; } static bool classof(const Stmt *S) { diff --git a/clang/lib/AST/StmtDumper.cpp b/clang/lib/AST/StmtDumper.cpp index 87a9f8e36b6d..358098eace72 100644 --- a/clang/lib/AST/StmtDumper.cpp +++ b/clang/lib/AST/StmtDumper.cpp @@ -405,9 +405,11 @@ void StmtDumper::VisitBinaryOperator(BinaryOperator *Node) { } void StmtDumper::VisitCompoundAssignOperator(CompoundAssignOperator *Node) { DumpExpr(Node); - fprintf(F, " '%s' ComputeTy=", + fprintf(F, " '%s' ComputeLHSTy=", BinaryOperator::getOpcodeStr(Node->getOpcode())); - DumpType(Node->getComputationType()); + DumpType(Node->getComputationLHSType()); + fprintf(F, " ComputeResultTy="); + DumpType(Node->getComputationResultType()); } // GNU extensions. diff --git a/clang/lib/AST/StmtSerialization.cpp b/clang/lib/AST/StmtSerialization.cpp index 518431146614..0d920fa23e3f 100644 --- a/clang/lib/AST/StmtSerialization.cpp +++ b/clang/lib/AST/StmtSerialization.cpp @@ -456,7 +456,8 @@ CharacterLiteral* CharacterLiteral::CreateImpl(Deserializer& D, ASTContext& C) { void CompoundAssignOperator::EmitImpl(Serializer& S) const { S.Emit(getType()); - S.Emit(ComputationType); + S.Emit(ComputationLHSType); + S.Emit(ComputationResultType); S.Emit(getOperatorLoc()); S.EmitInt(getOpcode()); S.BatchEmitOwnedPtrs(getLHS(),getRHS()); @@ -465,14 +466,15 @@ void CompoundAssignOperator::EmitImpl(Serializer& S) const { CompoundAssignOperator* CompoundAssignOperator::CreateImpl(Deserializer& D, ASTContext& C) { QualType t = QualType::ReadVal(D); - QualType c = QualType::ReadVal(D); + QualType cl = QualType::ReadVal(D); + QualType cr = QualType::ReadVal(D); SourceLocation L = SourceLocation::ReadVal(D); Opcode Opc = static_cast(D.ReadInt()); Expr* LHS, *RHS; D.BatchReadOwnedPtrs(LHS, RHS, C); return new (C, llvm::alignof()) - CompoundAssignOperator(LHS,RHS,Opc,t,c,L); + CompoundAssignOperator(LHS,RHS,Opc,t,cl,cr,L); } void CompoundLiteralExpr::EmitImpl(Serializer& S) const { diff --git a/clang/lib/Analysis/GRExprEngine.cpp b/clang/lib/Analysis/GRExprEngine.cpp index 2f3f0bf5ccbc..70e6647c27ee 100644 --- a/clang/lib/Analysis/GRExprEngine.cpp +++ b/clang/lib/Analysis/GRExprEngine.cpp @@ -2810,16 +2810,18 @@ void GRExprEngine::VisitBinaryOperator(BinaryOperator* B, // The RHS is not Unknown. // Get the computation type. - QualType CTy = cast(B)->getComputationType(); + QualType CTy = cast(B)->getComputationResultType(); CTy = getContext().getCanonicalType(CTy); - + + QualType CLHSTy = cast(B)->getComputationLHSType(); + CLHSTy = getContext().getCanonicalType(CTy); + QualType LTy = getContext().getCanonicalType(LHS->getType()); QualType RTy = getContext().getCanonicalType(RHS->getType()); - - // Perform promotions. - if (LTy != CTy) V = EvalCast(V, CTy); - if (RTy != CTy) RightV = EvalCast(RightV, CTy); - + + // Promote LHS. + V = EvalCast(V, CLHSTy); + // Evaluate operands and promote to result type. if (RightV.isUndef()) { // Propagate undefined values (right-side). diff --git a/clang/lib/CodeGen/CGExprComplex.cpp b/clang/lib/CodeGen/CGExprComplex.cpp index 3e4410611c51..3e28490f6ffa 100644 --- a/clang/lib/CodeGen/CGExprComplex.cpp +++ b/clang/lib/CodeGen/CGExprComplex.cpp @@ -419,7 +419,7 @@ EmitCompoundAssign(const CompoundAssignOperator *E, LValue LHSLV = CGF.EmitLValue(E->getLHS()); BinOpInfo OpInfo; - OpInfo.Ty = E->getComputationType(); + OpInfo.Ty = E->getComputationResultType(); // We know the LHS is a complex lvalue. OpInfo.LHS = EmitLoadOfComplex(LHSLV.getAddress(), LHSLV.isVolatileQualified()); diff --git a/clang/lib/CodeGen/CGExprScalar.cpp b/clang/lib/CodeGen/CGExprScalar.cpp index 89f28bea6d34..4f2371c4a189 100644 --- a/clang/lib/CodeGen/CGExprScalar.cpp +++ b/clang/lib/CodeGen/CGExprScalar.cpp @@ -768,76 +768,31 @@ Value *ScalarExprEmitter::EmitCompoundAssign(const CompoundAssignOperator *E, BinOpInfo OpInfo; - // Load the LHS and RHS operands. + if (E->getComputationResultType()->isAnyComplexType()) { + // FIXME: This needs to go through the complex expression emitter, but + // it's a tad complicated to do that... I'm leaving it out for now. + // (Note that we do actually need the imaginary part of the RHS for + // multiplication and division.) + CGF.ErrorUnsupported(E, "complex compound assignment"); + return llvm::UndefValue::get(CGF.ConvertType(E->getType())); + } + + // Load/convert the LHS. LValue LHSLV = EmitLValue(E->getLHS()); OpInfo.LHS = EmitLoadOfLValue(LHSLV, LHSTy); - - // Determine the computation type. If the RHS is complex, then this is one of - // the add/sub/mul/div operators. All of these operators can be computed in - // with just their real component even though the computation domain really is - // complex. - QualType ComputeType = E->getComputationType(); - - // If the computation type is complex, then the RHS is complex. Emit the RHS. - if (const ComplexType *CT = ComputeType->getAsComplexType()) { - ComputeType = CT->getElementType(); - - // Emit the RHS, only keeping the real component. - OpInfo.RHS = CGF.EmitComplexExpr(E->getRHS()).first; - RHSTy = RHSTy->getAsComplexType()->getElementType(); - } else { - // Otherwise the RHS is a simple scalar value. - OpInfo.RHS = Visit(E->getRHS()); - } - - QualType LComputeTy, RComputeTy, ResultTy; - - // Compound assignment does not contain enough information about all - // the types involved for pointer arithmetic cases. Figure it out - // here for now. - if (E->getLHS()->getType()->isPointerType()) { - // Pointer arithmetic cases: ptr +=,-= int and ptr -= ptr, - assert((E->getOpcode() == BinaryOperator::AddAssign || - E->getOpcode() == BinaryOperator::SubAssign) && - "Invalid compound assignment operator on pointer type."); - LComputeTy = E->getLHS()->getType(); - - if (E->getRHS()->getType()->isPointerType()) { - // Degenerate case of (ptr -= ptr) allowed by GCC implicit cast - // extension, the conversion from the pointer difference back to - // the LHS type is handled at the end. - assert(E->getOpcode() == BinaryOperator::SubAssign && - "Invalid compound assignment operator on pointer type."); - RComputeTy = E->getLHS()->getType(); - ResultTy = CGF.getContext().getPointerDiffType(); - } else { - RComputeTy = E->getRHS()->getType(); - ResultTy = LComputeTy; - } - } else if (E->getRHS()->getType()->isPointerType()) { - // Degenerate case of (int += ptr) allowed by GCC implicit cast - // extension. - assert(E->getOpcode() == BinaryOperator::AddAssign && - "Invalid compound assignment operator on pointer type."); - LComputeTy = E->getLHS()->getType(); - RComputeTy = E->getRHS()->getType(); - ResultTy = RComputeTy; - } else { - LComputeTy = RComputeTy = ResultTy = ComputeType; - } - - // Convert the LHS/RHS values to the computation type. - OpInfo.LHS = EmitScalarConversion(OpInfo.LHS, LHSTy, LComputeTy); - OpInfo.RHS = EmitScalarConversion(OpInfo.RHS, RHSTy, RComputeTy); - OpInfo.Ty = ResultTy; + OpInfo.LHS = EmitScalarConversion(OpInfo.LHS, LHSTy, + E->getComputationLHSType()); + // Emit the RHS. + OpInfo.RHS = Visit(E->getRHS()); + OpInfo.Ty = E->getComputationResultType(); OpInfo.E = E; // Expand the binary operator. Value *Result = (this->*Func)(OpInfo); // Convert the result back to the LHS type. - Result = EmitScalarConversion(Result, ResultTy, LHSTy); - + Result = EmitScalarConversion(Result, E->getComputationResultType(), LHSTy); + // Store the result value into the LHS lvalue. Bit-fields are // handled specially because the result is altered by the store, // i.e., [C99 6.5.16p1] 'An assignment expression has the value of diff --git a/clang/lib/Sema/Sema.h b/clang/lib/Sema/Sema.h index 76740e8dd99c..37a1799898b1 100644 --- a/clang/lib/Sema/Sema.h +++ b/clang/lib/Sema/Sema.h @@ -2297,9 +2297,9 @@ public: QualType CheckRemainderOperands( // C99 6.5.5 Expr *&lex, Expr *&rex, SourceLocation OpLoc, bool isCompAssign = false); QualType CheckAdditionOperands( // C99 6.5.6 - Expr *&lex, Expr *&rex, SourceLocation OpLoc, bool isCompAssign = false); + Expr *&lex, Expr *&rex, SourceLocation OpLoc, QualType* CompLHSTy = 0); QualType CheckSubtractionOperands( // C99 6.5.6 - Expr *&lex, Expr *&rex, SourceLocation OpLoc, bool isCompAssign = false); + Expr *&lex, Expr *&rex, SourceLocation OpLoc, QualType* CompLHSTy = 0); QualType CheckShiftOperands( // C99 6.5.7 Expr *&lex, Expr *&rex, SourceLocation OpLoc, bool isCompAssign = false); QualType CheckCompareOperands( // C99 6.5.8/9 diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp index 8b646e9943f3..0a77c21875f6 100644 --- a/clang/lib/Sema/SemaExpr.cpp +++ b/clang/lib/Sema/SemaExpr.cpp @@ -174,10 +174,10 @@ void Sema::DefaultVariadicArgumentPromotion(Expr *&Expr, VariadicCallType CT) { /// GCC. QualType Sema::UsualArithmeticConversions(Expr *&lhsExpr, Expr *&rhsExpr, bool isCompAssign) { - if (!isCompAssign) { + if (!isCompAssign) UsualUnaryConversions(lhsExpr); - UsualUnaryConversions(rhsExpr); - } + + UsualUnaryConversions(rhsExpr); // For conversion purposes, we ignore any qualifiers. // For example, "const float" and "float" are equivalent. @@ -196,10 +196,9 @@ QualType Sema::UsualArithmeticConversions(Expr *&lhsExpr, Expr *&rhsExpr, return lhs; QualType destType = UsualArithmeticConversionsType(lhs, rhs); - if (!isCompAssign) { + if (!isCompAssign) ImpCastExprToType(lhsExpr, destType); - ImpCastExprToType(rhsExpr, destType); - } + ImpCastExprToType(rhsExpr, destType); return destType; } @@ -3147,16 +3146,22 @@ inline QualType Sema::CheckRemainderOperands( } inline QualType Sema::CheckAdditionOperands( // C99 6.5.6 - Expr *&lex, Expr *&rex, SourceLocation Loc, bool isCompAssign) + Expr *&lex, Expr *&rex, SourceLocation Loc, QualType* CompLHSTy) { - if (lex->getType()->isVectorType() || rex->getType()->isVectorType()) - return CheckVectorOperands(Loc, lex, rex); + if (lex->getType()->isVectorType() || rex->getType()->isVectorType()) { + QualType compType = CheckVectorOperands(Loc, lex, rex); + if (CompLHSTy) *CompLHSTy = compType; + return compType; + } - QualType compType = UsualArithmeticConversions(lex, rex, isCompAssign); + QualType compType = UsualArithmeticConversions(lex, rex, CompLHSTy); // handle the common case first (both operands are arithmetic). - if (lex->getType()->isArithmeticType() && rex->getType()->isArithmeticType()) + if (lex->getType()->isArithmeticType() && + rex->getType()->isArithmeticType()) { + if (CompLHSTy) *CompLHSTy = compType; return compType; + } // Put any potential pointer into PExp Expr* PExp = lex, *IExp = rex; @@ -3193,6 +3198,12 @@ inline QualType Sema::CheckAdditionOperands( // C99 6.5.6 lex->getType())) return QualType(); + if (CompLHSTy) { + QualType LHSTy = lex->getType(); + if (LHSTy->isPromotableIntegerType()) + LHSTy = Context.IntTy; + *CompLHSTy = LHSTy; + } return PExp->getType(); } } @@ -3202,17 +3213,22 @@ inline QualType Sema::CheckAdditionOperands( // C99 6.5.6 // C99 6.5.6 QualType Sema::CheckSubtractionOperands(Expr *&lex, Expr *&rex, - SourceLocation Loc, bool isCompAssign) { - if (lex->getType()->isVectorType() || rex->getType()->isVectorType()) - return CheckVectorOperands(Loc, lex, rex); + SourceLocation Loc, QualType* CompLHSTy) { + if (lex->getType()->isVectorType() || rex->getType()->isVectorType()) { + QualType compType = CheckVectorOperands(Loc, lex, rex); + if (CompLHSTy) *CompLHSTy = compType; + return compType; + } - QualType compType = UsualArithmeticConversions(lex, rex, isCompAssign); + QualType compType = UsualArithmeticConversions(lex, rex, CompLHSTy); // Enforce type constraints: C99 6.5.6p3. // Handle the common case first (both operands are arithmetic). - if (lex->getType()->isArithmeticType() && rex->getType()->isArithmeticType()) + if (lex->getType()->isArithmeticType() && rex->getType()->isArithmeticType()) { + if (CompLHSTy) *CompLHSTy = compType; return compType; + } // Either ptr - int or ptr - ptr. if (const PointerType *LHSPTy = lex->getType()->getAsPointerType()) { @@ -3258,6 +3274,7 @@ QualType Sema::CheckSubtractionOperands(Expr *&lex, Expr *&rex, << ComplainAboutFunc->getType() << ComplainAboutFunc->getSourceRange(); + if (CompLHSTy) *CompLHSTy = lex->getType(); return lex->getType(); } @@ -3310,7 +3327,8 @@ QualType Sema::CheckSubtractionOperands(Expr *&lex, Expr *&rex, Diag(Loc, diag::ext_gnu_ptr_func_arith) << ComplainAboutFunc->getType() << ComplainAboutFunc->getSourceRange(); - + + if (CompLHSTy) *CompLHSTy = lex->getType(); return Context.getPointerDiffType(); } } @@ -3327,12 +3345,18 @@ QualType Sema::CheckShiftOperands(Expr *&lex, Expr *&rex, SourceLocation Loc, // Shifts don't perform usual arithmetic conversions, they just do integer // promotions on each operand. C99 6.5.7p3 + QualType LHSTy; + if (lex->getType()->isPromotableIntegerType()) + LHSTy = Context.IntTy; + else + LHSTy = lex->getType(); if (!isCompAssign) - UsualUnaryConversions(lex); + ImpCastExprToType(lex, LHSTy); + UsualUnaryConversions(rex); // "The type of the result is that of the promoted left operand." - return lex->getType(); + return LHSTy; } // C99 6.5.8 @@ -4049,9 +4073,11 @@ static inline UnaryOperator::Opcode ConvertTokenKindToUnaryOpcode( Action::OwningExprResult Sema::CreateBuiltinBinOp(SourceLocation OpLoc, unsigned Op, Expr *lhs, Expr *rhs) { - QualType ResultTy; // Result type of the binary operator. - QualType CompTy; // Computation type for compound assignments (e.g. '+=') + QualType ResultTy; // Result type of the binary operator. BinaryOperator::Opcode Opc = (BinaryOperator::Opcode)Op; + // The following two variables are used for compound assignment operators + QualType CompLHSTy; // Type of LHS after promotions for computation + QualType CompResultTy; // Type of computation result switch (Opc) { case BinaryOperator::Assign: @@ -4100,37 +4126,41 @@ Action::OwningExprResult Sema::CreateBuiltinBinOp(SourceLocation OpLoc, break; case BinaryOperator::MulAssign: case BinaryOperator::DivAssign: - CompTy = CheckMultiplyDivideOperands(lhs, rhs, OpLoc, true); - if (!CompTy.isNull()) - ResultTy = CheckAssignmentOperands(lhs, rhs, OpLoc, CompTy); + CompResultTy = CheckMultiplyDivideOperands(lhs, rhs, OpLoc, true); + CompLHSTy = CompResultTy; + if (!CompResultTy.isNull()) + ResultTy = CheckAssignmentOperands(lhs, rhs, OpLoc, CompResultTy); break; case BinaryOperator::RemAssign: - CompTy = CheckRemainderOperands(lhs, rhs, OpLoc, true); - if (!CompTy.isNull()) - ResultTy = CheckAssignmentOperands(lhs, rhs, OpLoc, CompTy); + CompResultTy = CheckRemainderOperands(lhs, rhs, OpLoc, true); + CompLHSTy = CompResultTy; + if (!CompResultTy.isNull()) + ResultTy = CheckAssignmentOperands(lhs, rhs, OpLoc, CompResultTy); break; case BinaryOperator::AddAssign: - CompTy = CheckAdditionOperands(lhs, rhs, OpLoc, true); - if (!CompTy.isNull()) - ResultTy = CheckAssignmentOperands(lhs, rhs, OpLoc, CompTy); + CompResultTy = CheckAdditionOperands(lhs, rhs, OpLoc, &CompLHSTy); + if (!CompResultTy.isNull()) + ResultTy = CheckAssignmentOperands(lhs, rhs, OpLoc, CompResultTy); break; case BinaryOperator::SubAssign: - CompTy = CheckSubtractionOperands(lhs, rhs, OpLoc, true); - if (!CompTy.isNull()) - ResultTy = CheckAssignmentOperands(lhs, rhs, OpLoc, CompTy); + CompResultTy = CheckSubtractionOperands(lhs, rhs, OpLoc, &CompLHSTy); + if (!CompResultTy.isNull()) + ResultTy = CheckAssignmentOperands(lhs, rhs, OpLoc, CompResultTy); break; case BinaryOperator::ShlAssign: case BinaryOperator::ShrAssign: - CompTy = CheckShiftOperands(lhs, rhs, OpLoc, true); - if (!CompTy.isNull()) - ResultTy = CheckAssignmentOperands(lhs, rhs, OpLoc, CompTy); + CompResultTy = CheckShiftOperands(lhs, rhs, OpLoc, true); + CompLHSTy = CompResultTy; + if (!CompResultTy.isNull()) + ResultTy = CheckAssignmentOperands(lhs, rhs, OpLoc, CompResultTy); break; case BinaryOperator::AndAssign: case BinaryOperator::XorAssign: case BinaryOperator::OrAssign: - CompTy = CheckBitwiseOperands(lhs, rhs, OpLoc, true); - if (!CompTy.isNull()) - ResultTy = CheckAssignmentOperands(lhs, rhs, OpLoc, CompTy); + CompResultTy = CheckBitwiseOperands(lhs, rhs, OpLoc, true); + CompLHSTy = CompResultTy; + if (!CompResultTy.isNull()) + ResultTy = CheckAssignmentOperands(lhs, rhs, OpLoc, CompResultTy); break; case BinaryOperator::Comma: ResultTy = CheckCommaOperands(lhs, rhs, OpLoc); @@ -4138,11 +4168,12 @@ Action::OwningExprResult Sema::CreateBuiltinBinOp(SourceLocation OpLoc, } if (ResultTy.isNull()) return ExprError(); - if (CompTy.isNull()) + if (CompResultTy.isNull()) return Owned(new (Context) BinaryOperator(lhs, rhs, Opc, ResultTy, OpLoc)); else return Owned(new (Context) CompoundAssignOperator(lhs, rhs, Opc, ResultTy, - CompTy, OpLoc)); + CompLHSTy, CompResultTy, + OpLoc)); } // Binary Operators. 'Tok' is the token for the operator. diff --git a/clang/test/CodeGen/complex.c b/clang/test/CodeGen/complex.c index 75fae79715e6..6a0d3d628c54 100644 --- a/clang/test/CodeGen/complex.c +++ b/clang/test/CodeGen/complex.c @@ -32,7 +32,8 @@ void test3() { double Gr = __real g1; cf += D; - D += cf; + // FIXME: Currently unsupported! + //D += cf; cf /= g1; g1 = g1 + D; g1 = D + g1; diff --git a/clang/test/CodeGen/compound-type.c b/clang/test/CodeGen/compound-type.c new file mode 100644 index 000000000000..352f6cc3ce65 --- /dev/null +++ b/clang/test/CodeGen/compound-type.c @@ -0,0 +1,7 @@ +// RUN: clang-cc < %s -emit-llvm -triple i686-pc-linux-gnu > %t && +// RUN: grep "div i32" %t && +// RUN: grep "shl i32" %t + +unsigned char a,b; +void c(void) {a <<= b;} +void d(void) {a /= b;}