aaron.ballman created this revision.
We do not explicitly model integer promotions on unary operators even though
those promotions happen in practice. This patch tracks the most important piece
of information about the promotion on the AST node: whether the operator can
overflow or not. It then pulls this logic out from various places and instead
uses what's calculated on the AST node.
This is effectively a NFC patch, however, it does help out-of-tree builds that
need to know about the integral promotions.
https://reviews.llvm.org/D33563
Files:
include/clang/AST/Expr.h
lib/AST/ASTDumper.cpp
lib/AST/ASTImporter.cpp
lib/AST/ExprConstant.cpp
lib/CodeGen/CGExprScalar.cpp
lib/CodeGen/CGStmtOpenMP.cpp
lib/Sema/SemaDeclCXX.cpp
lib/Sema/SemaExpr.cpp
lib/Sema/SemaPseudoObject.cpp
lib/Serialization/ASTReaderStmt.cpp
lib/Serialization/ASTWriterStmt.cpp
test/Misc/ast-dump-stmt.c
Index: test/Misc/ast-dump-stmt.c
===================================================================
--- test/Misc/ast-dump-stmt.c
+++ test/Misc/ast-dump-stmt.c
@@ -33,3 +33,35 @@
// CHECK-NEXT: OpaqueValueExpr
// CHECK-NEXT: IntegerLiteral
// CHECK-NEXT: IntegerLiteral
+
+void TestUnaryOperatorExpr(void) {
+ char T1 = 1;
+ int T2 = 1;
+
+ T1++;
+ T2++;
+ // CHECK: UnaryOperator{{.*}}postfix '++' cannot overflow
+ // CHECK-NEXT: DeclRefExpr{{.*}}'T1' 'char'
+ // CHECK-NOT: UnaryOperator{{.*}}postfix '++' cannot overflow
+ // CHECK: DeclRefExpr{{.*}}'T2' 'int'
+
+ -T1;
+ -T2;
+ // CHECK: UnaryOperator{{.*}}prefix '-' cannot overflow
+ // CHECK-NEXT: ImplicitCastExpr
+ // CHECK-NEXT: ImplicitCastExpr
+ // CHECK-NEXT: DeclRefExpr{{.*}}'T1' 'char'
+ // CHECK-NOT: UnaryOperator{{.*}}prefix '-' cannot overflow
+ // CHECK: ImplicitCastExpr
+ // CHECK: DeclRefExpr{{.*}}'T2' 'int'
+
+ ~T1;
+ ~T2;
+ // CHECK: UnaryOperator{{.*}}prefix '~' cannot overflow
+ // CHECK-NEXT: ImplicitCastExpr
+ // CHECK-NEXT: ImplicitCastExpr
+ // CHECK-NEXT: DeclRefExpr{{.*}}'T1' 'char'
+ // CHECK-NOT: UnaryOperator{{.*}}prefix '~' cannot overflow
+ // CHECK: ImplicitCastExpr
+ // CHECK: DeclRefExpr{{.*}}'T2' 'int'
+}
Index: lib/Serialization/ASTWriterStmt.cpp
===================================================================
--- lib/Serialization/ASTWriterStmt.cpp
+++ lib/Serialization/ASTWriterStmt.cpp
@@ -490,6 +490,7 @@
Record.AddStmt(E->getSubExpr());
Record.push_back(E->getOpcode()); // FIXME: stable encoding
Record.AddSourceLocation(E->getOperatorLoc());
+ Record.push_back(E->canOverflow());
Code = serialization::EXPR_UNARY_OPERATOR;
}
Index: lib/Serialization/ASTReaderStmt.cpp
===================================================================
--- lib/Serialization/ASTReaderStmt.cpp
+++ lib/Serialization/ASTReaderStmt.cpp
@@ -536,6 +536,7 @@
E->setSubExpr(Record.readSubExpr());
E->setOpcode((UnaryOperator::Opcode)Record.readInt());
E->setOperatorLoc(ReadSourceLocation());
+ E->setCanOverflow(Record.readInt());
}
void ASTStmtReader::VisitOffsetOfExpr(OffsetOfExpr *E) {
Index: lib/Sema/SemaPseudoObject.cpp
===================================================================
--- lib/Sema/SemaPseudoObject.cpp
+++ lib/Sema/SemaPseudoObject.cpp
@@ -527,9 +527,12 @@
(result.get()->isTypeDependent() || CanCaptureValue(result.get())))
setResultToLastSemantic();
- UnaryOperator *syntactic =
- new (S.Context) UnaryOperator(syntacticOp, opcode, resultType,
- VK_LValue, OK_Ordinary, opcLoc);
+ UnaryOperator *syntactic = new (S.Context) UnaryOperator(
+ syntacticOp, opcode, resultType, VK_LValue, OK_Ordinary, opcLoc,
+ !resultType->isDependentType()
+ ? S.Context.getTypeSize(resultType) >=
+ S.Context.getTypeSize(S.Context.IntTy)
+ : false);
return complete(syntactic);
}
@@ -1639,9 +1642,9 @@
Expr *syntax = E->getSyntacticForm();
if (UnaryOperator *uop = dyn_cast<UnaryOperator>(syntax)) {
Expr *op = stripOpaqueValuesFromPseudoObjectRef(*this, uop->getSubExpr());
- return new (Context) UnaryOperator(op, uop->getOpcode(), uop->getType(),
- uop->getValueKind(), uop->getObjectKind(),
- uop->getOperatorLoc());
+ return new (Context) UnaryOperator(
+ op, uop->getOpcode(), uop->getType(), uop->getValueKind(),
+ uop->getObjectKind(), uop->getOperatorLoc(), uop->canOverflow());
} else if (CompoundAssignOperator *cop
= dyn_cast<CompoundAssignOperator>(syntax)) {
Expr *lhs = stripOpaqueValuesFromPseudoObjectRef(*this, cop->getLHS());
Index: lib/Sema/SemaExpr.cpp
===================================================================
--- lib/Sema/SemaExpr.cpp
+++ lib/Sema/SemaExpr.cpp
@@ -11860,6 +11860,16 @@
return CreateBuiltinBinOp(OpLoc, Opc, LHSExpr, RHSExpr);
}
+static bool isOverflowingIntegerType(ASTContext &Ctx, QualType T) {
+ if (T.isNull() || T->isDependentType())
+ return false;
+
+ if (!T->isPromotableIntegerType())
+ return true;
+
+ return Ctx.getIntWidth(T) >= Ctx.getIntWidth(Ctx.IntTy);
+}
+
ExprResult Sema::CreateBuiltinUnaryOp(SourceLocation OpLoc,
UnaryOperatorKind Opc,
Expr *InputExpr) {
@@ -11867,6 +11877,8 @@
ExprValueKind VK = VK_RValue;
ExprObjectKind OK = OK_Ordinary;
QualType resultType;
+ bool CanOverflow = false;
+
if (getLangOpts().OpenCL) {
QualType Ty = InputExpr->getType();
// The only legal unary operation for atomics is '&'.
@@ -11891,6 +11903,7 @@
Opc == UO_PostInc,
Opc == UO_PreInc ||
Opc == UO_PreDec);
+ CanOverflow = isOverflowingIntegerType(Context, resultType);
break;
case UO_AddrOf:
resultType = CheckAddressOfOperand(Input, OpLoc);
@@ -11904,9 +11917,11 @@
}
case UO_Plus:
case UO_Minus:
+ CanOverflow = isOverflowingIntegerType(Context, Input.get()->getType());
Input = UsualUnaryConversions(Input.get());
if (Input.isInvalid()) return ExprError();
resultType = Input.get()->getType();
+
if (resultType->isDependentType())
break;
if (resultType->isArithmeticType()) // C99 6.5.3.3p1
@@ -11926,10 +11941,12 @@
<< resultType << Input.get()->getSourceRange());
case UO_Not: // bitwise complement
+ CanOverflow = isOverflowingIntegerType(Context, Input.get()->getType());
Input = UsualUnaryConversions(Input.get());
if (Input.isInvalid())
return ExprError();
resultType = Input.get()->getType();
+
if (resultType->isDependentType())
break;
// C99 6.5.3.3p1. We allow complex int and float as a GCC extension.
@@ -12041,7 +12058,7 @@
CheckArrayAccess(Input.get());
return new (Context)
- UnaryOperator(Input.get(), Opc, resultType, VK, OK, OpLoc);
+ UnaryOperator(Input.get(), Opc, resultType, VK, OK, OpLoc, CanOverflow);
}
/// \brief Determine whether the given expression is a qualified member
Index: lib/Sema/SemaDeclCXX.cpp
===================================================================
--- lib/Sema/SemaDeclCXX.cpp
+++ lib/Sema/SemaDeclCXX.cpp
@@ -11113,9 +11113,9 @@
VK_RValue, OK_Ordinary, Loc, FPOptions());
// Create the pre-increment of the iteration variable.
- Expr *Increment
- = new (S.Context) UnaryOperator(IterationVarRef.build(S, Loc), UO_PreInc,
- SizeType, VK_LValue, OK_Ordinary, Loc);
+ Expr *Increment = new (S.Context)
+ UnaryOperator(IterationVarRef.build(S, Loc), UO_PreInc, SizeType,
+ VK_LValue, OK_Ordinary, Loc, true);
// Construct the loop that copies all elements of this array.
return S.ActOnForStmt(
Index: lib/CodeGen/CGStmtOpenMP.cpp
===================================================================
--- lib/CodeGen/CGStmtOpenMP.cpp
+++ lib/CodeGen/CGStmtOpenMP.cpp
@@ -2580,7 +2580,7 @@
OK_Ordinary, S.getLocStart(), FPOptions());
// Increment for loop counter.
UnaryOperator Inc(&IVRefExpr, UO_PreInc, KmpInt32Ty, VK_RValue, OK_Ordinary,
- S.getLocStart());
+ S.getLocStart(), true);
auto BodyGen = [Stmt, CS, &S, &IV](CodeGenFunction &CGF) {
// Iterate through all sections and emit a switch construct:
// switch (IV) {
Index: lib/CodeGen/CGExprScalar.cpp
===================================================================
--- lib/CodeGen/CGExprScalar.cpp
+++ lib/CodeGen/CGExprScalar.cpp
@@ -150,7 +150,7 @@
// If a unary op has a widened operand, the op cannot overflow.
if (const auto *UO = dyn_cast<UnaryOperator>(Op.E))
- return IsWidenedIntegerOp(Ctx, UO->getSubExpr());
+ return !UO->canOverflow();
// We usually don't need overflow checks for binops with widened operands.
// Multiplication with promoted unsigned operands is a special case.
@@ -1763,7 +1763,7 @@
}
case CK_IntToOCLSampler:
- return CGF.CGM.createOpenCLIntToSamplerConversion(E, CGF);
+ return CGF.CGM.createOpenCLIntToSamplerConversion(E, CGF);
} // end of switch
@@ -1819,7 +1819,7 @@
return Builder.CreateNSWAdd(InVal, Amount, Name);
// Fall through.
case LangOptions::SOB_Trapping:
- if (IsWidenedIntegerOp(CGF.getContext(), E->getSubExpr()))
+ if (!E->canOverflow())
return Builder.CreateNSWAdd(InVal, Amount, Name);
return EmitOverflowCheckedBinOp(createBinOpInfoFromIncDec(E, InVal, IsInc));
}
@@ -1900,11 +1900,9 @@
} else if (type->isIntegerType()) {
// Note that signed integer inc/dec with width less than int can't
// overflow because of promotion rules; we're just eliding a few steps here.
- bool CanOverflow = value->getType()->getIntegerBitWidth() >=
- CGF.IntTy->getIntegerBitWidth();
- if (CanOverflow && type->isSignedIntegerOrEnumerationType()) {
+ if (E->canOverflow() && type->isSignedIntegerOrEnumerationType()) {
value = EmitIncDecConsiderOverflowBehavior(E, value, isInc);
- } else if (CanOverflow && type->isUnsignedIntegerType() &&
+ } else if (E->canOverflow() && type->isUnsignedIntegerType() &&
CGF.SanOpts.has(SanitizerKind::UnsignedIntegerOverflow)) {
value =
EmitOverflowCheckedBinOp(createBinOpInfoFromIncDec(E, value, isInc));
Index: lib/AST/ExprConstant.cpp
===================================================================
--- lib/AST/ExprConstant.cpp
+++ lib/AST/ExprConstant.cpp
@@ -3157,11 +3157,6 @@
return Obj && modifySubobject(Info, E, Obj, LVal.Designator, Val);
}
-static bool isOverflowingIntegerType(ASTContext &Ctx, QualType T) {
- return T->isSignedIntegerType() &&
- Ctx.getIntWidth(T) >= Ctx.getIntWidth(Ctx.IntTy);
-}
-
namespace {
struct CompoundAssignSubobjectHandler {
EvalInfo &Info;
@@ -3283,7 +3278,7 @@
namespace {
struct IncDecSubobjectHandler {
EvalInfo &Info;
- const Expr *E;
+ const UnaryOperator *E;
AccessKinds AccessKind;
APValue *Old;
@@ -3355,8 +3350,7 @@
if (AccessKind == AK_Increment) {
++Value;
- if (!WasNegative && Value.isNegative() &&
- isOverflowingIntegerType(Info.Ctx, SubobjType)) {
+ if (!WasNegative && Value.isNegative() && E->canOverflow()) {
APSInt ActualValue(Value, /*IsUnsigned*/true);
return HandleOverflow(Info, E, ActualValue, SubobjType);
}
@@ -3363,8 +3357,7 @@
} else {
--Value;
- if (WasNegative && !Value.isNegative() &&
- isOverflowingIntegerType(Info.Ctx, SubobjType)) {
+ if (WasNegative && !Value.isNegative() && E->canOverflow()) {
unsigned BitWidth = Value.getBitWidth();
APSInt ActualValue(Value.sext(BitWidth + 1), /*IsUnsigned*/false);
ActualValue.setBit(BitWidth);
@@ -3425,7 +3418,7 @@
AccessKinds AK = IsIncrement ? AK_Increment : AK_Decrement;
CompleteObject Obj = findCompleteObject(Info, E, AK, LVal, LValType);
- IncDecSubobjectHandler Handler = { Info, E, AK, Old };
+ IncDecSubobjectHandler Handler = {Info, cast<UnaryOperator>(E), AK, Old};
return Obj && findSubobject(Info, E, Obj, LVal.Designator, Handler);
}
@@ -8804,7 +8797,7 @@
return false;
if (!Result.isInt()) return Error(E);
const APSInt &Value = Result.getInt();
- if (Value.isSigned() && Value.isMinSignedValue() &&
+ if (Value.isSigned() && Value.isMinSignedValue() && E->canOverflow() &&
!HandleOverflow(Info, E, -Value.extend(Value.getBitWidth() + 1),
E->getType()))
return false;
Index: lib/AST/ASTImporter.cpp
===================================================================
--- lib/AST/ASTImporter.cpp
+++ lib/AST/ASTImporter.cpp
@@ -4658,14 +4658,13 @@
if (!SubExpr)
return nullptr;
- return new (Importer.getToContext()) UnaryOperator(SubExpr, E->getOpcode(),
- T, E->getValueKind(),
- E->getObjectKind(),
- Importer.Import(E->getOperatorLoc()));
+ return new (Importer.getToContext()) UnaryOperator(
+ SubExpr, E->getOpcode(), T, E->getValueKind(), E->getObjectKind(),
+ Importer.Import(E->getOperatorLoc()), E->canOverflow());
}
-Expr *ASTNodeImporter::VisitUnaryExprOrTypeTraitExpr(
- UnaryExprOrTypeTraitExpr *E) {
+Expr *
+ASTNodeImporter::VisitUnaryExprOrTypeTraitExpr(UnaryExprOrTypeTraitExpr *E) {
QualType ResultType = Importer.Import(E->getType());
if (E->isArgumentType()) {
Index: lib/AST/ASTDumper.cpp
===================================================================
--- lib/AST/ASTDumper.cpp
+++ lib/AST/ASTDumper.cpp
@@ -2047,6 +2047,8 @@
VisitExpr(Node);
OS << " " << (Node->isPostfix() ? "postfix" : "prefix")
<< " '" << UnaryOperator::getOpcodeStr(Node->getOpcode()) << "'";
+ if (!Node->canOverflow())
+ OS << " cannot overflow";
}
void ASTDumper::VisitUnaryExprOrTypeTraitExpr(
Index: include/clang/AST/Expr.h
===================================================================
--- include/clang/AST/Expr.h
+++ include/clang/AST/Expr.h
@@ -1717,20 +1717,20 @@
private:
unsigned Opc : 5;
+ unsigned CanOverflow : 1;
SourceLocation Loc;
Stmt *Val;
public:
+ UnaryOperator(Expr *input, Opcode opc, QualType type, ExprValueKind VK,
+ ExprObjectKind OK, SourceLocation l, bool CanOverflow = false)
+ : Expr(UnaryOperatorClass, type, VK, OK,
+ input->isTypeDependent() || type->isDependentType(),
+ input->isValueDependent(),
+ (input->isInstantiationDependent() ||
+ type->isInstantiationDependentType()),
+ input->containsUnexpandedParameterPack()),
+ Opc(opc), CanOverflow(CanOverflow), Loc(l), Val(input) {}
- UnaryOperator(Expr *input, Opcode opc, QualType type,
- ExprValueKind VK, ExprObjectKind OK, SourceLocation l)
- : Expr(UnaryOperatorClass, type, VK, OK,
- input->isTypeDependent() || type->isDependentType(),
- input->isValueDependent(),
- (input->isInstantiationDependent() ||
- type->isInstantiationDependentType()),
- input->containsUnexpandedParameterPack()),
- Opc(opc), Loc(l), Val(input) {}
-
/// \brief Build an empty unary operator.
explicit UnaryOperator(EmptyShell Empty)
: Expr(UnaryOperatorClass, Empty), Opc(UO_AddrOf) { }
@@ -1745,6 +1745,15 @@
SourceLocation getOperatorLoc() const { return Loc; }
void setOperatorLoc(SourceLocation L) { Loc = L; }
+ /// Returns true if the unary operator can cause an overflow. For instance,
+ /// signed int i = INT_MAX; i++;
+ /// signed char c = CHAR_MAX; c++;
+ /// Due to integer promotions, c++ is promoted to an int before the postfix
+ /// increment, and the result is an int that cannot overflow. However, i++
+ /// can overflow.
+ bool canOverflow() const { return CanOverflow; }
+ void setCanOverflow(bool C) { CanOverflow = C; }
+
/// isPostfix - Return true if this is a postfix operation, like x++.
static bool isPostfix(Opcode Op) {
return Op == UO_PostInc || Op == UO_PostDec;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
