Skip to content

Commit 34376a6

Browse files
rjmccallrjmccall
committed
Although we currently have explicit lvalue-to-rvalue conversions, they're
not actually frequently used, because ImpCastExprToType only creates a node if the types differ. So explicitly create an ICE in the lvalue-to-rvalue conversion code in DefaultFunctionArrayLvalueConversion() as well as several other new places, and consistently deal with the consequences throughout the compiler. In addition, introduce a new cast kind for loading an ObjCProperty l-value, and make sure we emit those nodes whenever an ObjCProperty l-value appears that's not on the LHS of an assignment operator. This breaks a couple of rewriter tests, which I've x-failed until future development occurs on the rewriter. Ted Kremenek kindly contributed the analyzer workarounds in this patch. llvm-svn: 120890
1 parent 1c8ac8f commit 34376a6

30 files changed

+419
-111
lines changed

‎clang/include/clang/AST/Expr.h

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ namespace clang {
3939
class CXXBaseSpecifier;
4040
class CXXOperatorCallExpr;
4141
class CXXMemberCallExpr;
42+
class ObjCPropertyRefExpr;
4243
class TemplateArgumentLoc;
4344
class TemplateArgumentListInfo;
4445

@@ -312,6 +313,10 @@ class Expr : public Stmt {
312313
return const_cast<Expr*>(this)->getBitField();
313314
}
314315

316+
/// \brief If this expression is an l-value for an Objective C
317+
/// property, find the underlying property reference expression.
318+
const ObjCPropertyRefExpr *getObjCProperty() const;
319+
315320
/// \brief Returns whether this expression refers to a vector element.
316321
bool refersToVectorElement() const;
317322

@@ -456,6 +461,14 @@ class Expr : public Stmt {
456461
const Expr *IgnoreParenImpCasts() const {
457462
return const_cast<Expr*>(this)->IgnoreParenImpCasts();
458463
}
464+
465+
/// Ignore parentheses and lvalue casts. Strip off any ParenExpr and
466+
/// CastExprs that represent lvalue casts, returning their operand.
467+
Expr *IgnoreParenLValueCasts();
468+
469+
const Expr *IgnoreParenLValueCasts() const {
470+
return const_cast<Expr*>(this)->IgnoreParenLValueCasts();
471+
}
459472

460473
/// IgnoreParenNoopCasts - Ignore parentheses and casts that do not change the
461474
/// value (including ptr->int casts of the same size). Strip off any
@@ -2056,6 +2069,7 @@ class CastExpr : public Expr {
20562069

20572070
case CK_Dependent:
20582071
case CK_LValueToRValue:
2072+
case CK_GetObjCProperty:
20592073
case CK_NoOp:
20602074
case CK_PointerToBoolean:
20612075
case CK_IntegralToBoolean:

‎clang/include/clang/AST/OperationKinds.h

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,13 @@ enum CastKind {
4848
/// an r-value from the operand gl-value. The result of an r-value
4949
/// conversion is always unqualified.
5050
CK_LValueToRValue,
51+
52+
/// CK_GetObjCProperty - A conversion which calls an Objective-C
53+
/// property getter. The operand is an OK_ObjCProperty l-value; the
54+
/// result will generally be an r-value, but could be an ordinary
55+
/// gl-value if the property reference is to an implicit property
56+
/// for a method that returns a reference type.
57+
CK_GetObjCProperty,
5158

5259
/// CK_NoOp - A conversion which does not affect the type other than
5360
/// (possibly) adding qualifiers.

‎clang/include/clang/Sema/Sema.h

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1579,6 +1579,7 @@ class Sema {
15791579
SourceLocation StartLoc,
15801580
SourceLocation EndLoc);
15811581
void ActOnForEachDeclStmt(DeclGroupPtrTy Decl);
1582+
StmtResult ActOnForEachLValueExpr(Expr *E);
15821583
StmtResult ActOnCaseStmt(SourceLocation CaseLoc, Expr *LHSVal,
15831584
SourceLocation DotDotDotLoc, Expr *RHSVal,
15841585
SourceLocation ColonLoc);
@@ -3984,6 +3985,11 @@ class Sema {
39843985
ExprValueKind VK = VK_RValue,
39853986
const CXXCastPath *BasePath = 0);
39863987

3988+
/// IgnoredValueConversions - Given that an expression's result is
3989+
/// syntactically ignored, perform any conversions that are
3990+
/// required.
3991+
void IgnoredValueConversions(Expr *&expr);
3992+
39873993
// UsualUnaryConversions - promotes integers (C99 6.3.1.1p2) and converts
39883994
// functions and arrays to their respective pointers (C99 6.3.2.1).
39893995
Expr *UsualUnaryConversions(Expr *&expr);
@@ -4190,7 +4196,8 @@ class Sema {
41904196
QualType CheckAssignmentOperands( // C99 6.5.16.[1,2]
41914197
Expr *lex, Expr *&rex, SourceLocation OpLoc, QualType convertedType);
41924198

4193-
void ConvertPropertyAssignment(Expr *LHS, Expr *&RHS, QualType& LHSTy);
4199+
void ConvertPropertyForRValue(Expr *&E);
4200+
void ConvertPropertyForLValue(Expr *&LHS, Expr *&RHS, QualType& LHSTy);
41944201

41954202
QualType CheckConditionalOperands( // C99 6.5.15
41964203
Expr *&cond, Expr *&lhs, Expr *&rhs, Expr *&save,

‎clang/lib/AST/Expr.cpp

Lines changed: 44 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -824,6 +824,8 @@ const char *CastExpr::getCastKindName() const {
824824
return "LValueBitCast";
825825
case CK_LValueToRValue:
826826
return "LValueToRValue";
827+
case CK_GetObjCProperty:
828+
return "GetObjCProperty";
827829
case CK_NoOp:
828830
return "NoOp";
829831
case CK_BaseToDerived:
@@ -1722,6 +1724,24 @@ Expr *Expr::IgnoreParenCasts() {
17221724
}
17231725
}
17241726

1727+
Expr *Expr::IgnoreParenLValueCasts() {
1728+
Expr *E = this;
1729+
while (E) {
1730+
if (ParenExpr *P = dyn_cast<ParenExpr>(E)) {
1731+
E = P->getSubExpr();
1732+
continue;
1733+
}
1734+
if (CastExpr *P = dyn_cast<CastExpr>(E)) {
1735+
if (P->getCastKind() == CK_LValueToRValue) {
1736+
E = P->getSubExpr();
1737+
continue;
1738+
}
1739+
}
1740+
break;
1741+
}
1742+
return E;
1743+
}
1744+
17251745
Expr *Expr::IgnoreParenImpCasts() {
17261746
Expr *E = this;
17271747
while (true) {
@@ -2043,12 +2063,34 @@ bool Expr::isNullPointerConstant(ASTContext &Ctx,
20432063
return isIntegerConstantExpr(Result, Ctx) && Result == 0;
20442064
}
20452065

2066+
/// \brief If this expression is an l-value for an Objective C
2067+
/// property, find the underlying property reference expression.
2068+
const ObjCPropertyRefExpr *Expr::getObjCProperty() const {
2069+
const Expr *E = this;
2070+
while (true) {
2071+
assert((E->getValueKind() == VK_LValue &&
2072+
E->getObjectKind() == OK_ObjCProperty) &&
2073+
"expression is not a property reference");
2074+
E = E->IgnoreParenCasts();
2075+
if (const BinaryOperator *BO = dyn_cast<BinaryOperator>(E)) {
2076+
if (BO->getOpcode() == BO_Comma) {
2077+
E = BO->getRHS();
2078+
continue;
2079+
}
2080+
}
2081+
2082+
break;
2083+
}
2084+
2085+
return cast<ObjCPropertyRefExpr>(E);
2086+
}
2087+
20462088
FieldDecl *Expr::getBitField() {
20472089
Expr *E = this->IgnoreParens();
20482090

20492091
while (ImplicitCastExpr *ICE = dyn_cast<ImplicitCastExpr>(E)) {
2050-
if (ICE->getValueKind() != VK_RValue &&
2051-
ICE->getCastKind() == CK_NoOp)
2092+
if (ICE->getCastKind() == CK_LValueToRValue ||
2093+
(ICE->getValueKind() != VK_RValue && ICE->getCastKind() == CK_NoOp))
20522094
E = ICE->getSubExpr()->IgnoreParens();
20532095
else
20542096
break;

‎clang/lib/AST/ExprClassification.cpp

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -413,8 +413,10 @@ static Cl::Kinds ClassifyBinaryOp(ASTContext &Ctx, const BinaryOperator *E) {
413413
assert(Ctx.getLangOptions().CPlusPlus &&
414414
"This is only relevant for C++.");
415415
// C++ [expr.ass]p1: All [...] return an lvalue referring to the left operand.
416+
// Except we override this for writes to ObjC properties.
416417
if (E->isAssignmentOp())
417-
return Cl::CL_LValue;
418+
return (E->getLHS()->getObjectKind() == OK_ObjCProperty
419+
? Cl::CL_PRValue : Cl::CL_LValue);
418420

419421
// C++ [expr.comma]p1: the result is of the same value category as its right
420422
// operand, [...].
@@ -468,9 +470,10 @@ static Cl::ModifiableType IsModifiable(ASTContext &Ctx, const Expr *E,
468470
if (Kind == Cl::CL_PRValue) {
469471
// For the sake of better diagnostics, we want to specifically recognize
470472
// use of the GCC cast-as-lvalue extension.
471-
if (const CStyleCastExpr *CE = dyn_cast<CStyleCastExpr>(E->IgnoreParens())){
472-
if (CE->getSubExpr()->Classify(Ctx).isLValue()) {
473-
Loc = CE->getLParenLoc();
473+
if (const ExplicitCastExpr *CE =
474+
dyn_cast<ExplicitCastExpr>(E->IgnoreParens())) {
475+
if (CE->getSubExpr()->IgnoreParenImpCasts()->isLValue()) {
476+
Loc = CE->getExprLoc();
474477
return Cl::CM_LValueCast;
475478
}
476479
}

‎clang/lib/Analysis/CFG.cpp

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -825,6 +825,16 @@ CFGBlock* CFGBuilder::Visit(Stmt * S, AddStmtChoice asc) {
825825

826826
case Stmt::ContinueStmtClass:
827827
return VisitContinueStmt(cast<ContinueStmt>(S));
828+
829+
case Stmt::CStyleCastExprClass: {
830+
CastExpr *castExpr = cast<CastExpr>(S);
831+
if (castExpr->getCastKind() == CK_LValueToRValue) {
832+
// temporary workaround
833+
S = castExpr->getSubExpr();
834+
goto tryAgain;
835+
}
836+
return VisitStmt(S, asc);
837+
}
828838

829839
case Stmt::CXXCatchStmtClass:
830840
return VisitCXXCatchStmt(cast<CXXCatchStmt>(S));
@@ -871,8 +881,15 @@ CFGBlock* CFGBuilder::Visit(Stmt * S, AddStmtChoice asc) {
871881
case Stmt::IfStmtClass:
872882
return VisitIfStmt(cast<IfStmt>(S));
873883

874-
case Stmt::ImplicitCastExprClass:
875-
return VisitImplicitCastExpr(cast<ImplicitCastExpr>(S), asc);
884+
case Stmt::ImplicitCastExprClass: {
885+
ImplicitCastExpr *castExpr = cast<ImplicitCastExpr>(S);
886+
if (castExpr->getCastKind() == CK_LValueToRValue) {
887+
// temporary workaround
888+
S = castExpr->getSubExpr();
889+
goto tryAgain;
890+
}
891+
return VisitImplicitCastExpr(castExpr, asc);
892+
}
876893

877894
case Stmt::IndirectGotoStmtClass:
878895
return VisitIndirectGotoStmt(cast<IndirectGotoStmt>(S));

‎clang/lib/Checker/CheckSecuritySyntaxOnly.cpp

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -187,8 +187,10 @@ void WalkAST::CheckLoopConditionForFloat(const ForStmt *FS) {
187187
return;
188188

189189
// Are we comparing variables?
190-
const DeclRefExpr *drLHS = dyn_cast<DeclRefExpr>(B->getLHS()->IgnoreParens());
191-
const DeclRefExpr *drRHS = dyn_cast<DeclRefExpr>(B->getRHS()->IgnoreParens());
190+
const DeclRefExpr *drLHS =
191+
dyn_cast<DeclRefExpr>(B->getLHS()->IgnoreParenLValueCasts());
192+
const DeclRefExpr *drRHS =
193+
dyn_cast<DeclRefExpr>(B->getRHS()->IgnoreParenLValueCasts());
192194

193195
// Does at least one of the variables have a floating point type?
194196
drLHS = drLHS && drLHS->getType()->isRealFloatingType() ? drLHS : NULL;

‎clang/lib/Checker/DereferenceChecker.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,7 @@ void DereferenceChecker::AddDerefSource(llvm::raw_ostream &os,
5959
llvm::SmallVectorImpl<SourceRange> &Ranges,
6060
const Expr *Ex,
6161
bool loadedFrom) {
62+
Ex = Ex->IgnoreParenLValueCasts();
6263
switch (Ex->getStmtClass()) {
6364
default:
6465
return;

‎clang/lib/Checker/Environment.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,8 @@ SVal Environment::getSVal(const Stmt *E, SValBuilder& svalBuilder) const {
5353
QualType CT = C->getType();
5454
if (CT->isVoidType())
5555
return UnknownVal();
56-
if (C->getCastKind() == CK_NoOp) {
56+
if (C->getCastKind() == CK_NoOp ||
57+
C->getCastKind() == CK_LValueToRValue) { // temporary workaround
5758
E = C->getSubExpr();
5859
continue;
5960
}

‎clang/lib/Checker/GRExprEngine.cpp

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -778,6 +778,10 @@ void GRExprEngine::Visit(const Stmt* S, ExplodedNode* Pred,
778778
S->getLocStart(),
779779
"Error evaluating statement");
780780

781+
// Expressions to ignore.
782+
if (const Expr *Ex = dyn_cast<Expr>(S))
783+
S = Ex->IgnoreParenLValueCasts();
784+
781785
// FIXME: add metadata to the CFG so that we can disable
782786
// this check when we KNOW that there is no block-level subexpression.
783787
// The motivation is that this check requires a hashtable lookup.
@@ -814,7 +818,9 @@ void GRExprEngine::Visit(const Stmt* S, ExplodedNode* Pred,
814818
MakeNode(Dst, S, Pred, GetState(Pred));
815819
break;
816820
}
817-
821+
822+
case Stmt::ParenExprClass:
823+
llvm_unreachable("ParenExprs already handled.");
818824
// Cases that should never be evaluated simply because they shouldn't
819825
// appear in the CFG.
820826
case Stmt::BreakStmtClass:
@@ -1039,10 +1045,6 @@ void GRExprEngine::Visit(const Stmt* S, ExplodedNode* Pred,
10391045
break;
10401046
}
10411047

1042-
case Stmt::ParenExprClass:
1043-
Visit(cast<ParenExpr>(S)->getSubExpr()->IgnoreParens(), Pred, Dst);
1044-
break;
1045-
10461048
case Stmt::ReturnStmtClass:
10471049
VisitReturnStmt(cast<ReturnStmt>(S), Pred, Dst);
10481050
break;
@@ -1113,8 +1115,8 @@ void GRExprEngine::VisitLValue(const Expr* Ex, ExplodedNode* Pred,
11131115
Ex->getLocStart(),
11141116
"Error evaluating statement");
11151117

1116-
1117-
Ex = Ex->IgnoreParens();
1118+
// Expressions to ignore.
1119+
Ex = Ex->IgnoreParenLValueCasts();
11181120

11191121
if (Ex != CurrentStmt && Pred->getLocationContext()->getCFG()->isBlkExpr(Ex)){
11201122
Dst.Add(Pred);
@@ -2661,6 +2663,7 @@ void GRExprEngine::VisitCast(const CastExpr *CastE, const Expr *Ex,
26612663
}
26622664
return;
26632665

2666+
case CK_GetObjCProperty:
26642667
case CK_Dependent:
26652668
case CK_ArrayToPointerDecay:
26662669
case CK_BitCast:

‎clang/lib/Checker/IdempotentOperationChecker.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -543,7 +543,7 @@ bool IdempotentOperationChecker::isTruncationExtensionAssignment(
543543
if (VD != RHS_DR->getDecl())
544544
return false;
545545

546-
return dyn_cast<DeclRefExpr>(RHS->IgnoreParens()) == NULL;
546+
return dyn_cast<DeclRefExpr>(RHS->IgnoreParenLValueCasts()) == NULL;
547547
}
548548

549549
// Returns false if a path to this block was not completely analyzed, or true

‎clang/lib/CodeGen/CGExpr.cpp

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1755,6 +1755,21 @@ LValue CodeGenFunction::EmitCastLValue(const CastExpr *E) {
17551755
case CK_Dependent:
17561756
llvm_unreachable("dependent cast kind in IR gen!");
17571757

1758+
case CK_GetObjCProperty: {
1759+
LValue LV = EmitLValue(E->getSubExpr());
1760+
assert(LV.isPropertyRef());
1761+
RValue RV = EmitLoadOfPropertyRefLValue(LV);
1762+
1763+
// Property is an aggregate r-value.
1764+
if (RV.isAggregate()) {
1765+
return MakeAddrLValue(RV.getAggregateAddr(), E->getType());
1766+
}
1767+
1768+
// Implicit property returns an l-value.
1769+
assert(RV.isScalar());
1770+
return MakeAddrLValue(RV.getScalarVal(), E->getSubExpr()->getType());
1771+
}
1772+
17581773
case CK_NoOp:
17591774
if (!E->getSubExpr()->isRValue() || E->getType()->isRecordType()) {
17601775
LValue LV = EmitLValue(E->getSubExpr());

‎clang/lib/CodeGen/CGExprAgg.cpp

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -282,8 +282,16 @@ void AggExprEmitter::VisitCastExpr(CastExpr *E) {
282282
break;
283283
}
284284

285+
case CK_GetObjCProperty: {
286+
LValue LV = CGF.EmitLValue(E->getSubExpr());
287+
assert(LV.isPropertyRef());
288+
RValue RV = CGF.EmitLoadOfPropertyRefLValue(LV, getReturnValueSlot());
289+
EmitGCMove(E, RV);
290+
break;
291+
}
292+
293+
case CK_LValueToRValue: // hope for downstream optimization
285294
case CK_NoOp:
286-
case CK_LValueToRValue:
287295
case CK_UserDefinedConversion:
288296
case CK_ConstructorConversion:
289297
assert(CGF.getContext().hasSameUnqualifiedType(E->getSubExpr()->getType(),
@@ -349,9 +357,8 @@ void AggExprEmitter::VisitObjCMessageExpr(ObjCMessageExpr *E) {
349357
}
350358

351359
void AggExprEmitter::VisitObjCPropertyRefExpr(ObjCPropertyRefExpr *E) {
352-
RValue RV = CGF.EmitLoadOfPropertyRefLValue(CGF.EmitObjCPropertyRefLValue(E),
353-
getReturnValueSlot());
354-
EmitGCMove(E, RV);
360+
llvm_unreachable("direct property access not surrounded by "
361+
"lvalue-to-rvalue cast");
355362
}
356363

357364
void AggExprEmitter::VisitBinComma(const BinaryOperator *E) {

‎clang/lib/CodeGen/CGExprComplex.cpp

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,7 @@ class ComplexExprEmitter
108108
return EmitLoadOfLValue(E);
109109
}
110110
ComplexPairTy VisitObjCPropertyRefExpr(ObjCPropertyRefExpr *E) {
111+
assert(E->getObjectKind() == OK_Ordinary);
111112
return EmitLoadOfLValue(E);
112113
}
113114
ComplexPairTy VisitObjCMessageExpr(ObjCMessageExpr *E) {
@@ -333,7 +334,21 @@ ComplexPairTy ComplexExprEmitter::EmitComplexToComplexCast(ComplexPairTy Val,
333334

334335
ComplexPairTy ComplexExprEmitter::EmitCast(CastExpr::CastKind CK, Expr *Op,
335336
QualType DestTy) {
336-
// FIXME: this should be based off of the CastKind.
337+
switch (CK) {
338+
case CK_GetObjCProperty: {
339+
LValue LV = CGF.EmitLValue(Op);
340+
assert(LV.isPropertyRef() && "Unknown LValue type!");
341+
return CGF.EmitLoadOfPropertyRefLValue(LV).getComplexVal();
342+
}
343+
344+
case CK_NoOp:
345+
case CK_LValueToRValue:
346+
return Visit(Op);
347+
348+
// TODO: do all of these
349+
default:
350+
break;
351+
}
337352

338353
// Two cases here: cast from (complex to complex) and (scalar to complex).
339354
if (Op->getType()->isAnyComplexType())

0 commit comments

Comments
 (0)