From a5e660188a3c654cf0c88ed1093b28207e870b2b Mon Sep 17 00:00:00 2001 From: Eli Friedman Date: Sat, 20 Jul 2013 00:40:58 +0000 Subject: [PATCH] Make IgnoreParens() look through ChooseExprs. This is the same way GenericSelectionExpr works, and it's generally a more consistent approach. A large part of this patch is devoted to caching the value of the condition of a ChooseExpr; it's needed to avoid threading an ASTContext into IgnoreParens(). Fixes . git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@186738 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/clang/AST/EvaluatedExprVisitor.h | 2 +- include/clang/AST/Expr.h | 21 +++- lib/AST/Expr.cpp | 99 +++++-------------- lib/AST/ExprClassification.cpp | 2 +- lib/AST/ExprConstant.cpp | 4 +- lib/CodeGen/CGExpr.cpp | 2 +- lib/CodeGen/CGExprAgg.cpp | 2 +- lib/CodeGen/CGExprComplex.cpp | 2 +- lib/CodeGen/CGExprConstant.cpp | 2 +- lib/CodeGen/CGExprScalar.cpp | 2 +- lib/Sema/SemaExceptionSpec.cpp | 2 +- lib/Sema/SemaExpr.cpp | 6 +- lib/Sema/SemaPseudoObject.cpp | 19 ++++ lib/Serialization/ASTReaderStmt.cpp | 1 + lib/Serialization/ASTWriterStmt.cpp | 1 + .../Checkers/IdempotentOperationChecker.cpp | 15 +-- test/SemaObjC/property-choose-expr.m | 14 +++ 17 files changed, 92 insertions(+), 104 deletions(-) create mode 100644 test/SemaObjC/property-choose-expr.m diff --git a/include/clang/AST/EvaluatedExprVisitor.h b/include/clang/AST/EvaluatedExprVisitor.h index 2e3cbfad91..12c4fcc49b 100644 --- a/include/clang/AST/EvaluatedExprVisitor.h +++ b/include/clang/AST/EvaluatedExprVisitor.h @@ -53,7 +53,7 @@ public: if (E->getCond()->isValueDependent()) return; // Only the selected subexpression matters; the other one is not evaluated. - return this->Visit(E->getChosenSubExpr(Context)); + return this->Visit(E->getChosenSubExpr()); } void VisitDesignatedInitExpr(DesignatedInitExpr *E) { diff --git a/include/clang/AST/Expr.h b/include/clang/AST/Expr.h index d035fbdf66..42c99bbee0 100644 --- a/include/clang/AST/Expr.h +++ b/include/clang/AST/Expr.h @@ -3475,10 +3475,12 @@ class ChooseExpr : public Expr { enum { COND, LHS, RHS, END_EXPR }; Stmt* SubExprs[END_EXPR]; // Left/Middle/Right hand sides. SourceLocation BuiltinLoc, RParenLoc; + bool CondIsTrue; public: ChooseExpr(SourceLocation BLoc, Expr *cond, Expr *lhs, Expr *rhs, QualType t, ExprValueKind VK, ExprObjectKind OK, - SourceLocation RP, bool TypeDependent, bool ValueDependent) + SourceLocation RP, bool condIsTrue, + bool TypeDependent, bool ValueDependent) : Expr(ChooseExprClass, t, VK, OK, TypeDependent, ValueDependent, (cond->isInstantiationDependent() || lhs->isInstantiationDependent() || @@ -3486,7 +3488,7 @@ public: (cond->containsUnexpandedParameterPack() || lhs->containsUnexpandedParameterPack() || rhs->containsUnexpandedParameterPack())), - BuiltinLoc(BLoc), RParenLoc(RP) { + BuiltinLoc(BLoc), RParenLoc(RP), CondIsTrue(condIsTrue) { SubExprs[COND] = cond; SubExprs[LHS] = lhs; SubExprs[RHS] = rhs; @@ -3497,12 +3499,21 @@ public: /// isConditionTrue - Return whether the condition is true (i.e. not /// equal to zero). - bool isConditionTrue(const ASTContext &C) const; + bool isConditionTrue() const { + assert(!isConditionDependent() && + "Dependent condition isn't true or false"); + return CondIsTrue; + } + void setIsConditionTrue(bool isTrue) { CondIsTrue = isTrue; } + + bool isConditionDependent() const { + return getCond()->isTypeDependent() || getCond()->isValueDependent(); + } /// getChosenSubExpr - Return the subexpression chosen according to the /// condition. - Expr *getChosenSubExpr(const ASTContext &C) const { - return isConditionTrue(C) ? getLHS() : getRHS(); + Expr *getChosenSubExpr() const { + return isConditionTrue() ? getLHS() : getRHS(); } Expr *getCond() const { return cast(SubExprs[COND]); } diff --git a/lib/AST/Expr.cpp b/lib/AST/Expr.cpp index 37f7d267d9..8473c096af 100644 --- a/lib/AST/Expr.cpp +++ b/lib/AST/Expr.cpp @@ -1929,6 +1929,9 @@ bool Expr::isUnusedResultAWarning(const Expr *&WarnE, SourceLocation &Loc, case GenericSelectionExprClass: return cast(this)->getResultExpr()-> isUnusedResultAWarning(WarnE, Loc, R1, R2, Ctx); + case ChooseExprClass: + return cast(this)->getChosenSubExpr()-> + isUnusedResultAWarning(WarnE, Loc, R1, R2, Ctx); case UnaryOperatorClass: { const UnaryOperator *UO = cast(this); @@ -2295,6 +2298,12 @@ Expr* Expr::IgnoreParens() { continue; } } + if (ChooseExpr* P = dyn_cast(E)) { + if (!P->isConditionDependent()) { + E = P->getChosenSubExpr(); + continue; + } + } return E; } } @@ -2304,26 +2313,11 @@ Expr* Expr::IgnoreParens() { Expr *Expr::IgnoreParenCasts() { Expr *E = this; while (true) { - if (ParenExpr* P = dyn_cast(E)) { - E = P->getSubExpr(); - continue; - } + E = E->IgnoreParens(); if (CastExpr *P = dyn_cast(E)) { E = P->getSubExpr(); continue; } - if (UnaryOperator* P = dyn_cast(E)) { - if (P->getOpcode() == UO_Extension) { - E = P->getSubExpr(); - continue; - } - } - if (GenericSelectionExpr* P = dyn_cast(E)) { - if (!P->isResultDependent()) { - E = P->getResultExpr(); - continue; - } - } if (MaterializeTemporaryExpr *Materialize = dyn_cast(E)) { E = Materialize->GetTemporaryExpr(); @@ -2345,24 +2339,12 @@ Expr *Expr::IgnoreParenCasts() { Expr *Expr::IgnoreParenLValueCasts() { Expr *E = this; while (true) { - if (ParenExpr *P = dyn_cast(E)) { - E = P->getSubExpr(); - continue; - } else if (CastExpr *P = dyn_cast(E)) { + E = E->IgnoreParens(); + if (CastExpr *P = dyn_cast(E)) { if (P->getCastKind() == CK_LValueToRValue) { E = P->getSubExpr(); continue; } - } else if (UnaryOperator* P = dyn_cast(E)) { - if (P->getOpcode() == UO_Extension) { - E = P->getSubExpr(); - continue; - } - } else if (GenericSelectionExpr* P = dyn_cast(E)) { - if (!P->isResultDependent()) { - E = P->getResultExpr(); - continue; - } } else if (MaterializeTemporaryExpr *Materialize = dyn_cast(E)) { E = Materialize->GetTemporaryExpr(); @@ -2380,10 +2362,7 @@ Expr *Expr::IgnoreParenLValueCasts() { Expr *Expr::ignoreParenBaseCasts() { Expr *E = this; while (true) { - if (ParenExpr *P = dyn_cast(E)) { - E = P->getSubExpr(); - continue; - } + E = E->IgnoreParens(); if (CastExpr *CE = dyn_cast(E)) { if (CE->getCastKind() == CK_DerivedToBase || CE->getCastKind() == CK_UncheckedDerivedToBase || @@ -2400,26 +2379,11 @@ Expr *Expr::ignoreParenBaseCasts() { Expr *Expr::IgnoreParenImpCasts() { Expr *E = this; while (true) { - if (ParenExpr *P = dyn_cast(E)) { - E = P->getSubExpr(); - continue; - } + E = E->IgnoreParens(); if (ImplicitCastExpr *P = dyn_cast(E)) { E = P->getSubExpr(); continue; } - if (UnaryOperator* P = dyn_cast(E)) { - if (P->getOpcode() == UO_Extension) { - E = P->getSubExpr(); - continue; - } - } - if (GenericSelectionExpr* P = dyn_cast(E)) { - if (!P->isResultDependent()) { - E = P->getResultExpr(); - continue; - } - } if (MaterializeTemporaryExpr *Materialize = dyn_cast(E)) { E = Materialize->GetTemporaryExpr(); @@ -2448,10 +2412,7 @@ Expr *Expr::IgnoreConversionOperator() { Expr *Expr::IgnoreParenNoopCasts(ASTContext &Ctx) { Expr *E = this; while (true) { - if (ParenExpr *P = dyn_cast(E)) { - E = P->getSubExpr(); - continue; - } + E = E->IgnoreParens(); if (CastExpr *P = dyn_cast(E)) { // We ignore integer <-> casts that are of the same width, ptr<->ptr and @@ -2473,20 +2434,6 @@ Expr *Expr::IgnoreParenNoopCasts(ASTContext &Ctx) { } } - if (UnaryOperator* P = dyn_cast(E)) { - if (P->getOpcode() == UO_Extension) { - E = P->getSubExpr(); - continue; - } - } - - if (GenericSelectionExpr* P = dyn_cast(E)) { - if (!P->isResultDependent()) { - E = P->getResultExpr(); - continue; - } - } - if (SubstNonTypeTemplateParmExpr *NTTP = dyn_cast(E)) { E = NTTP->getReplacement(); @@ -2728,7 +2675,9 @@ bool Expr::isConstantInitializer(ASTContext &Ctx, bool IsForRef) const { return cast(this)->getResultExpr() ->isConstantInitializer(Ctx, IsForRef); case ChooseExprClass: - return cast(this)->getChosenSubExpr(Ctx) + if (cast(this)->isConditionDependent()) + return false; + return cast(this)->getChosenSubExpr() ->isConstantInitializer(Ctx, IsForRef); case UnaryOperatorClass: { const UnaryOperator* Exp = cast(this); @@ -2887,7 +2836,7 @@ bool Expr::HasSideEffects(const ASTContext &Ctx) const { HasSideEffects(Ctx); case ChooseExprClass: - return cast(this)->getChosenSubExpr(Ctx)->HasSideEffects(Ctx); + return cast(this)->getChosenSubExpr()->HasSideEffects(Ctx); case CXXDefaultArgExprClass: return cast(this)->getExpr()->HasSideEffects(Ctx); @@ -3082,7 +3031,13 @@ Expr::isNullPointerConstant(ASTContext &Ctx, return PE->getSubExpr()->isNullPointerConstant(Ctx, NPC); } else if (const GenericSelectionExpr *GE = dyn_cast(this)) { + if (GE->isResultDependent()) + return NPCK_NotNull; return GE->getResultExpr()->isNullPointerConstant(Ctx, NPC); + } else if (const ChooseExpr *CE = dyn_cast(this)) { + if (CE->isConditionDependent()) + return NPCK_NotNull; + return CE->getChosenSubExpr()->isNullPointerConstant(Ctx, NPC); } else if (const CXXDefaultArgExpr *DefaultArg = dyn_cast(this)) { // See through default argument expressions. @@ -3567,10 +3522,6 @@ StringRef ObjCBridgedCastExpr::getBridgeKindName() const { llvm_unreachable("Invalid BridgeKind!"); } -bool ChooseExpr::isConditionTrue(const ASTContext &C) const { - return getCond()->EvaluateKnownConstInt(C) != 0; -} - ShuffleVectorExpr::ShuffleVectorExpr(ASTContext &C, ArrayRef args, QualType Type, SourceLocation BLoc, SourceLocation RP) diff --git a/lib/AST/ExprClassification.cpp b/lib/AST/ExprClassification.cpp index 72f17669ed..1c91af7d47 100644 --- a/lib/AST/ExprClassification.cpp +++ b/lib/AST/ExprClassification.cpp @@ -286,7 +286,7 @@ static Cl::Kinds ClassifyInternal(ASTContext &Ctx, const Expr *E) { // __builtin_choose_expr is equivalent to the chosen expression. case Expr::ChooseExprClass: - return ClassifyInternal(Ctx, cast(E)->getChosenSubExpr(Ctx)); + return ClassifyInternal(Ctx, cast(E)->getChosenSubExpr()); // Extended vector element access is an lvalue unless there are duplicates // in the shuffle expression. diff --git a/lib/AST/ExprConstant.cpp b/lib/AST/ExprConstant.cpp index 28473f0bdb..ddfb64c44d 100644 --- a/lib/AST/ExprConstant.cpp +++ b/lib/AST/ExprConstant.cpp @@ -3585,7 +3585,7 @@ public: RetTy VisitUnaryPlus(const UnaryOperator *E) { return StmtVisitorTy::Visit(E->getSubExpr()); } RetTy VisitChooseExpr(const ChooseExpr *E) - { return StmtVisitorTy::Visit(E->getChosenSubExpr(Info.Ctx)); } + { return StmtVisitorTy::Visit(E->getChosenSubExpr()); } RetTy VisitGenericSelectionExpr(const GenericSelectionExpr *E) { return StmtVisitorTy::Visit(E->getResultExpr()); } RetTy VisitSubstNonTypeTemplateParmExpr(const SubstNonTypeTemplateParmExpr *E) @@ -8263,7 +8263,7 @@ static ICEDiag CheckICE(const Expr* E, ASTContext &Ctx) { case Expr::CXXDefaultInitExprClass: return CheckICE(cast(E)->getExpr(), Ctx); case Expr::ChooseExprClass: { - return CheckICE(cast(E)->getChosenSubExpr(Ctx), Ctx); + return CheckICE(cast(E)->getChosenSubExpr(), Ctx); } } diff --git a/lib/CodeGen/CGExpr.cpp b/lib/CodeGen/CGExpr.cpp index b283174514..d6ff93f5fb 100644 --- a/lib/CodeGen/CGExpr.cpp +++ b/lib/CodeGen/CGExpr.cpp @@ -851,7 +851,7 @@ LValue CodeGenFunction::EmitLValue(const Expr *E) { case Expr::BinaryConditionalOperatorClass: return EmitConditionalOperatorLValue(cast(E)); case Expr::ChooseExprClass: - return EmitLValue(cast(E)->getChosenSubExpr(getContext())); + return EmitLValue(cast(E)->getChosenSubExpr()); case Expr::OpaqueValueExprClass: return EmitOpaqueValueLValue(cast(E)); case Expr::SubstNonTypeTemplateParmExprClass: diff --git a/lib/CodeGen/CGExprAgg.cpp b/lib/CodeGen/CGExprAgg.cpp index 902bf1605a..8268c86c9a 100644 --- a/lib/CodeGen/CGExprAgg.cpp +++ b/lib/CodeGen/CGExprAgg.cpp @@ -920,7 +920,7 @@ VisitAbstractConditionalOperator(const AbstractConditionalOperator *E) { } void AggExprEmitter::VisitChooseExpr(const ChooseExpr *CE) { - Visit(CE->getChosenSubExpr(CGF.getContext())); + Visit(CE->getChosenSubExpr()); } void AggExprEmitter::VisitVAArgExpr(VAArgExpr *VE) { diff --git a/lib/CodeGen/CGExprComplex.cpp b/lib/CodeGen/CGExprComplex.cpp index 5ba13175d5..fcff8e9439 100644 --- a/lib/CodeGen/CGExprComplex.cpp +++ b/lib/CodeGen/CGExprComplex.cpp @@ -778,7 +778,7 @@ VisitAbstractConditionalOperator(const AbstractConditionalOperator *E) { } ComplexPairTy ComplexExprEmitter::VisitChooseExpr(ChooseExpr *E) { - return Visit(E->getChosenSubExpr(CGF.getContext())); + return Visit(E->getChosenSubExpr()); } ComplexPairTy ComplexExprEmitter::VisitInitListExpr(InitListExpr *E) { diff --git a/lib/CodeGen/CGExprConstant.cpp b/lib/CodeGen/CGExprConstant.cpp index 4a6f90a30e..431ee3c47f 100644 --- a/lib/CodeGen/CGExprConstant.cpp +++ b/lib/CodeGen/CGExprConstant.cpp @@ -610,7 +610,7 @@ public: } llvm::Constant *VisitChooseExpr(ChooseExpr *CE) { - return Visit(CE->getChosenSubExpr(CGM.getContext())); + return Visit(CE->getChosenSubExpr()); } llvm::Constant *VisitCompoundLiteralExpr(CompoundLiteralExpr *E) { diff --git a/lib/CodeGen/CGExprScalar.cpp b/lib/CodeGen/CGExprScalar.cpp index bfd880a7e3..7d98764548 100644 --- a/lib/CodeGen/CGExprScalar.cpp +++ b/lib/CodeGen/CGExprScalar.cpp @@ -3101,7 +3101,7 @@ VisitAbstractConditionalOperator(const AbstractConditionalOperator *E) { } Value *ScalarExprEmitter::VisitChooseExpr(ChooseExpr *E) { - return Visit(E->getChosenSubExpr(CGF.getContext())); + return Visit(E->getChosenSubExpr()); } Value *ScalarExprEmitter::VisitVAArgExpr(VAArgExpr *VE) { diff --git a/lib/Sema/SemaExceptionSpec.cpp b/lib/Sema/SemaExceptionSpec.cpp index 0385c7783e..0d662c8f39 100644 --- a/lib/Sema/SemaExceptionSpec.cpp +++ b/lib/Sema/SemaExceptionSpec.cpp @@ -1025,7 +1025,7 @@ CanThrowResult Sema::canThrow(const Expr *E) { case Expr::ChooseExprClass: if (E->isTypeDependent() || E->isValueDependent()) return CT_Dependent; - return canThrow(cast(E)->getChosenSubExpr(Context)); + return canThrow(cast(E)->getChosenSubExpr()); case Expr::GenericSelectionExprClass: if (cast(E)->isResultDependent()) diff --git a/lib/Sema/SemaExpr.cpp b/lib/Sema/SemaExpr.cpp index f397c5bb2e..2c933147f4 100644 --- a/lib/Sema/SemaExpr.cpp +++ b/lib/Sema/SemaExpr.cpp @@ -9868,6 +9868,7 @@ ExprResult Sema::ActOnChooseExpr(SourceLocation BuiltinLoc, ExprObjectKind OK = OK_Ordinary; QualType resType; bool ValueDependent = false; + bool CondIsTrue = false; if (CondExpr->isTypeDependent() || CondExpr->isValueDependent()) { resType = Context.DependentTy; ValueDependent = true; @@ -9880,9 +9881,10 @@ ExprResult Sema::ActOnChooseExpr(SourceLocation BuiltinLoc, if (CondICE.isInvalid()) return ExprError(); CondExpr = CondICE.take(); + CondIsTrue = condEval.getZExtValue(); // If the condition is > zero, then the AST type is the same as the LSHExpr. - Expr *ActiveExpr = condEval.getZExtValue() ? LHSExpr : RHSExpr; + Expr *ActiveExpr = CondIsTrue ? LHSExpr : RHSExpr; resType = ActiveExpr->getType(); ValueDependent = ActiveExpr->isValueDependent(); @@ -9891,7 +9893,7 @@ ExprResult Sema::ActOnChooseExpr(SourceLocation BuiltinLoc, } return Owned(new (Context) ChooseExpr(BuiltinLoc, CondExpr, LHSExpr, RHSExpr, - resType, VK, OK, RPLoc, + resType, VK, OK, RPLoc, CondIsTrue, resType->isDependentType(), ValueDependent)); } diff --git a/lib/Sema/SemaPseudoObject.cpp b/lib/Sema/SemaPseudoObject.cpp index a10612a8b0..106250fe0f 100644 --- a/lib/Sema/SemaPseudoObject.cpp +++ b/lib/Sema/SemaPseudoObject.cpp @@ -101,6 +101,25 @@ namespace { resultIndex); } + if (ChooseExpr *ce = dyn_cast(e)) { + assert(!ce->isConditionDependent()); + + Expr *LHS = ce->getLHS(), *RHS = ce->getRHS(); + Expr *&rebuiltExpr = ce->isConditionTrue() ? LHS : RHS; + rebuiltExpr = rebuild(rebuiltExpr); + + return new (S.Context) ChooseExpr(ce->getBuiltinLoc(), + ce->getCond(), + LHS, RHS, + rebuiltExpr->getType(), + rebuiltExpr->getValueKind(), + rebuiltExpr->getObjectKind(), + ce->getRParenLoc(), + ce->isConditionTrue(), + rebuiltExpr->isTypeDependent(), + rebuiltExpr->isValueDependent()); + } + llvm_unreachable("bad expression to rebuild!"); } }; diff --git a/lib/Serialization/ASTReaderStmt.cpp b/lib/Serialization/ASTReaderStmt.cpp index 113b12ff59..ed07b0532f 100644 --- a/lib/Serialization/ASTReaderStmt.cpp +++ b/lib/Serialization/ASTReaderStmt.cpp @@ -835,6 +835,7 @@ void ASTStmtReader::VisitChooseExpr(ChooseExpr *E) { E->setRHS(Reader.ReadSubExpr()); E->setBuiltinLoc(ReadSourceLocation(Record, Idx)); E->setRParenLoc(ReadSourceLocation(Record, Idx)); + E->setIsConditionTrue(Record[Idx++]); } void ASTStmtReader::VisitGNUNullExpr(GNUNullExpr *E) { diff --git a/lib/Serialization/ASTWriterStmt.cpp b/lib/Serialization/ASTWriterStmt.cpp index 8d47158f94..d8b852136c 100644 --- a/lib/Serialization/ASTWriterStmt.cpp +++ b/lib/Serialization/ASTWriterStmt.cpp @@ -773,6 +773,7 @@ void ASTStmtWriter::VisitChooseExpr(ChooseExpr *E) { Writer.AddStmt(E->getRHS()); Writer.AddSourceLocation(E->getBuiltinLoc(), Record); Writer.AddSourceLocation(E->getRParenLoc(), Record); + Record.push_back(E->isConditionDependent() ? false : E->isConditionTrue()); Code = serialization::EXPR_CHOOSE; } diff --git a/lib/StaticAnalyzer/Checkers/IdempotentOperationChecker.cpp b/lib/StaticAnalyzer/Checkers/IdempotentOperationChecker.cpp index 271ba4702c..d756c62956 100644 --- a/lib/StaticAnalyzer/Checkers/IdempotentOperationChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/IdempotentOperationChecker.cpp @@ -678,19 +678,8 @@ bool IdempotentOperationChecker::CanVary(const Expr *Ex, return CanVary(B->getRHS(), AC) || CanVary(B->getLHS(), AC); } - case Stmt::UnaryOperatorClass: { - const UnaryOperator *U = cast(Ex); - // Handle trivial case first - switch (U->getOpcode()) { - case UO_Extension: - return false; - default: - return CanVary(U->getSubExpr(), AC); - } - } - case Stmt::ChooseExprClass: - return CanVary(cast(Ex)->getChosenSubExpr( - AC->getASTContext()), AC); + case Stmt::UnaryOperatorClass: + return CanVary(cast(Ex)->getSubExpr(), AC); case Stmt::ConditionalOperatorClass: case Stmt::BinaryConditionalOperatorClass: return CanVary(cast(Ex)->getCond(), AC); diff --git a/test/SemaObjC/property-choose-expr.m b/test/SemaObjC/property-choose-expr.m new file mode 100644 index 0000000000..71265e5f8c --- /dev/null +++ b/test/SemaObjC/property-choose-expr.m @@ -0,0 +1,14 @@ +// RUN: %clang_cc1 -fsyntax-only -verify -Wno-objc-root-class %s +// expected-no-diagnostics + +@interface NSArray +-(int)count; +@end + +// +char* f(NSArray *array) { + return _Generic(__builtin_choose_expr(__builtin_types_compatible_p(__typeof__(array.count), void), 0.f, array.count), + unsigned int:"uint", + float:"void", + default: "ignored"); +} -- 2.40.0