[cfe-commits] [PATCH] Model C++11 rules for odr-use for variables correctly
Eli Friedman
eli.friedman at gmail.com
Fri Jan 27 19:56:35 PST 2012
On Fri, Jan 27, 2012 at 3:26 PM, Eli Friedman <eli.friedman at gmail.com> wrote:
> On Fri, Jan 27, 2012 at 3:12 PM, John McCall <rjmccall at apple.com> wrote:
>> On Jan 27, 2012, at 2:53 PM, Eli Friedman wrote:
>>> On Thu, Jan 26, 2012 at 7:29 PM, John McCall <rjmccall at apple.com> wrote:
>>>> On Jan 24, 2012, at 9:34 PM, Eli Friedman wrote:
>>>>> Patch attached. Basically, this is implementing the C++11 rules in
>>>>> [basic.def.odr]p2 for variables allowed in constant expressions.
>>>>> (Those rules don't matter so much for most code given how we do code
>>>>> generation, but we have to get the rules exactly right to get implicit
>>>>> capture in lambda expressions working correctly.)
>>>>>
>>>>> I don't really like this patch, but it seems like it is necessary.
>>>>
>>>> Yeah, I'm really not happy with this. Obvious alternative: mark ODR use
>>>> at the DRE creation site unless the object is not already marked and it
>>>> meets the constant-loading criteria. If it does, flag that this full-expression
>>>> contains a potential reference, then walk back over the completed
>>>> full-expression looking for PE DREs not direct operands of L2R casts.
>>>
>>> Is there any existing hook that actually triggers where I need it to?
>>> MaybeCreateExprWithCleanups seems close... but doesn't quite cover
>>> everything.
>>
>> What's missing?
>
> In essence, things which are constant-expressions in the C++ grammar
> are missing. Not too hard to add them, I guess.
New version attached; it doesn't pass regression tests because I still
need to add a hook to catch VLA array bounds attached to declarations,
but is this the direction you were thinking of?
-Eli
-------------- next part --------------
Index: include/clang/Sema/Sema.h
===================================================================
--- include/clang/Sema/Sema.h (revision 149031)
+++ include/clang/Sema/Sema.h (working copy)
@@ -2267,6 +2267,10 @@
void MarkDeclarationsReferencedInType(SourceLocation Loc, QualType T);
void MarkDeclarationsReferencedInExpr(Expr *E);
+ void MarkVarODRUsed(VarDecl *Var, SourceLocation Loc);
+ void FindUsesAsLValue(Stmt *S);
+ void MarkUseAsRValue(Expr *E);
+
/// \brief Try to recover by turning the given expression into a
/// call. Returns true if recovery was attempted or an error was
/// emitted; this may also leave the ExprResult invalid.
Index: lib/Sema/TreeTransform.h
===================================================================
--- lib/Sema/TreeTransform.h (revision 149031)
+++ lib/Sema/TreeTransform.h (working copy)
@@ -5313,6 +5313,7 @@
ExprResult Target = getDerived().TransformExpr(S->getTarget());
if (Target.isInvalid())
return StmtError();
+ Target = SemaRef.MaybeCreateExprWithCleanups(Target.take());
if (!getDerived().AlwaysRebuild() &&
Target.get() == S->getTarget())
@@ -5725,10 +5726,14 @@
ExprResult Cond = getDerived().TransformExpr(S->getCond());
if (Cond.isInvalid())
return StmtError();
+ if (Cond.get())
+ Cond = SemaRef.MaybeCreateExprWithCleanups(Cond.take());
ExprResult Inc = getDerived().TransformExpr(S->getInc());
if (Inc.isInvalid())
return StmtError();
+ if (Inc.get())
+ Inc = SemaRef.MaybeCreateExprWithCleanups(Inc.take());
StmtResult LoopVar = getDerived().TransformStmt(S->getLoopVarStmt());
if (LoopVar.isInvalid())
Index: lib/Sema/SemaExprCXX.cpp
===================================================================
--- lib/Sema/SemaExprCXX.cpp (revision 149003)
+++ lib/Sema/SemaExprCXX.cpp (working copy)
@@ -2051,6 +2051,7 @@
Condition = CheckBooleanCondition(Condition.take(), StmtLoc);
if (Condition.isInvalid())
return ExprError();
+ Condition = MaybeCreateExprWithCleanups(Condition);
}
return move(Condition);
@@ -4213,6 +4214,8 @@
Expr *Sema::MaybeCreateExprWithCleanups(Expr *SubExpr) {
assert(SubExpr && "sub expression can't be null!");
+ FindUsesAsLValue(SubExpr);
+
unsigned FirstCleanup = ExprEvalContexts.back().NumCleanupObjects;
assert(ExprCleanupObjects.size() >= FirstCleanup);
assert(ExprNeedsCleanups || ExprCleanupObjects.size() == FirstCleanup);
@@ -4232,6 +4235,8 @@
Stmt *Sema::MaybeCreateStmtWithCleanups(Stmt *SubStmt) {
assert(SubStmt && "sub statement can't be null!");
+ FindUsesAsLValue(SubStmt);
+
if (!ExprNeedsCleanups)
return SubStmt;
Index: lib/Sema/SemaTemplate.cpp
===================================================================
--- lib/Sema/SemaTemplate.cpp (revision 148998)
+++ lib/Sema/SemaTemplate.cpp (working copy)
@@ -3613,6 +3613,8 @@
// Create the template argument.
Converted = TemplateArgument(Entity->getCanonicalDecl());
S.MarkDeclarationReferenced(Arg->getLocStart(), Entity);
+ if (VarDecl *Var = dyn_cast<VarDecl>(Entity))
+ S.MarkVarODRUsed(Var, Arg->getLocStart());
return false;
}
Index: lib/Sema/SemaStmt.cpp
===================================================================
--- lib/Sema/SemaStmt.cpp (revision 149031)
+++ lib/Sema/SemaStmt.cpp (working copy)
@@ -513,14 +513,14 @@
if (CondResult.isInvalid()) return StmtError();
Cond = CondResult.take();
- if (!CondVar) {
+ if (!CondVar)
CheckImplicitConversions(Cond, SwitchLoc);
- CondResult = MaybeCreateExprWithCleanups(Cond);
- if (CondResult.isInvalid())
- return StmtError();
- Cond = CondResult.take();
- }
+ CondResult = MaybeCreateExprWithCleanups(Cond);
+ if (CondResult.isInvalid())
+ return StmtError();
+ Cond = CondResult.take();
+
getCurFunction()->setHasBranchIntoScope();
SwitchStmt *SS = new (Context) SwitchStmt(Context, ConditionVar, Cond);
@@ -1609,6 +1609,7 @@
E = ExprRes.take();
if (DiagnoseAssignmentResult(ConvTy, StarLoc, DestTy, ETy, E, AA_Passing))
return StmtError();
+ E = MaybeCreateExprWithCleanups(E);
}
getCurFunction()->setHasIndirectGoto();
Index: lib/Sema/SemaExpr.cpp
===================================================================
--- lib/Sema/SemaExpr.cpp (revision 149031)
+++ lib/Sema/SemaExpr.cpp (working copy)
@@ -9459,6 +9459,30 @@
return TranformToPotentiallyEvaluated(E);
}
+static bool DelayVarODRUsedForType(QualType T) {
+ // C++11 [basic.def.odr]p1: A variable or non-overloaded function whose name
+ // appears as a potentially-evaluated expression is odr-used unless it is
+ // an object that satisfies the requirements for appearing in a
+ // constant expression (5.19) and the lvalue-to-rvalue conversion (4.1)
+ // is immediately applied.
+
+ // We don't try to check the constant expression bit here because it gives
+ // our assertions better coverage.
+
+ // Check for an "object".
+ if (!T->isObjectType())
+ return false;
+ // The lvalue-to-rvalue conversion can't be applied to arrays.
+ if (T->isArrayType())
+ return false;
+ // lvalue-to-rvalue conversion doesn't apply to classes.
+ // FIXME: That isn't completely true... but I doubt anyone cares about the
+ // one edge case where we actually do apply lvalue-to-rvalue conversion.
+ if (T->isRecordType())
+ return false;
+ return true;
+}
+
/// \brief Note that the given declaration was referenced in the source code.
///
/// This routine should be invoke whenever a given declaration is referenced
@@ -9619,24 +9643,99 @@
}
}
- // Keep track of used but undefined variables. We make a hole in
- // the warning for static const data members with in-line
- // initializers.
- // FIXME: The hole we make for static const data members is too wide!
- // We need to implement the C++11 rules for odr-used.
- if (Var->hasDefinition() == VarDecl::DeclarationOnly
- && Var->getLinkage() != ExternalLinkage
- && !(Var->isStaticDataMember() && Var->hasInit())) {
- SourceLocation &old = UndefinedInternals[Var->getCanonicalDecl()];
- if (old.isInvalid()) old = Loc;
- }
+ // We delay actually treating the VarDecl as used in cases where it might
+ // not actually be odr-used.
+ if (!DelayVarODRUsedForType(Var->getType()))
+ MarkVarODRUsed(Var, Loc);
- D->setUsed(true);
return;
}
}
+void Sema::MarkVarODRUsed(VarDecl *Var, SourceLocation Loc) {
+ if (CurContext->isDependentContext())
+ return;
+
+ if (Var->isUsed(false))
+ return;
+
+ if (Var->hasDefinition() == VarDecl::DeclarationOnly &&
+ Var->getLinkage() != ExternalLinkage) {
+ SourceLocation &old = UndefinedInternals[Var->getCanonicalDecl()];
+ if (old.isInvalid()) old = Loc;
+ }
+
+ Var->setUsed(true);
+}
+
namespace {
+ /// \brief Helper class that marks all of the declarations referenced by
+ /// potentially-evaluated subexpressions as "referenced".
+ class MarkVarODRUses : public RecursiveASTVisitor<MarkVarODRUses> {
+ Sema &S;
+
+ public:
+ typedef RecursiveASTVisitor<MarkVarODRUses> Inherited;
+
+ explicit MarkVarODRUses(Sema &S) : S(S) { }
+
+ bool VisitDeclRefExpr(DeclRefExpr *E) {
+ VarDecl *Var = dyn_cast<VarDecl>(E->getDecl());
+ if (Var && DelayVarODRUsedForType(Var->getType()))
+ S.MarkVarODRUsed(Var, E->getLocation());
+ return true;
+ }
+
+ bool TraverseImplicitCastExpr(ImplicitCastExpr *E) {
+ if (E->getCastKind() == CK_LValueToRValue) {
+ Expr *SubExpr = E->getSubExpr()->IgnoreParens();
+ if (DeclRefExpr *DRE = dyn_cast<DeclRefExpr>(SubExpr)) {
+ VarDecl *Var = dyn_cast<VarDecl>(DRE->getDecl());
+ if (Var && DelayVarODRUsedForType(Var->getType()) &&
+ !DRE->isEvaluatable(S.Context))
+ S.MarkVarODRUsed(Var, DRE->getLocation());
+ return true;
+ }
+ }
+
+ return Inherited::TraverseImplicitCastExpr(E);
+ }
+
+ bool TraverseUnaryExprOrTypeTraitExpr(UnaryExprOrTypeTraitExpr *E) {
+ if (E->getKind() != UETT_SizeOf ||
+ !E->getTypeOfArgument()->isVariableArrayType())
+ return true;
+ return Inherited::TraverseUnaryExprOrTypeTraitExpr(E);
+ }
+ bool TraverseExpressionTraitExpr(ExpressionTraitExpr *E) { return true; }
+ bool TraverseCXXUuidofExpr(CXXUuidofExpr *E) { return true; }
+ bool TraverseCXXNoexceptExpr(CXXNoexceptExpr *E) { return true; }
+ bool TraverseChooseExpr(ChooseExpr *E) {
+ // Only the selected subexpression matters; the other one is not evaluated.
+ return TraverseStmt(E->getChosenSubExpr(S.Context));
+ }
+ bool TraverseCXXTypeidExpr(CXXTypeidExpr *E) {
+ // typeid(expression) is potentially evaluated when the argument is
+ // a glvalue of polymorphic type. (C++ 5.2.8p2-3)
+ if (!E->isTypeOperand() && E->Classify(S.Context).isGLValue())
+ if (const RecordType *Record
+ = E->getExprOperand()->getType()->getAs<RecordType>())
+ if (cast<CXXRecordDecl>(Record->getDecl())->isPolymorphic())
+ return Inherited::TraverseCXXTypeidExpr(E);
+ return true;
+ }
+ bool TraverseBlockExpr(BlockExpr *E) { return true; }
+ };
+}
+
+void Sema::FindUsesAsLValue(Stmt *S) {
+ if (CurContext->isDependentContext())
+ return;
+
+ MarkVarODRUses(*this).TraverseStmt(S);
+}
+
+namespace {
// Mark all of the declarations referenced
// FIXME: Not fully implemented yet! We need to have a better understanding
// of when we're entering
More information about the cfe-commits
mailing list