[cfe-commits] [PATCH] Model C++11 rules for odr-use for variables correctly
Eli Friedman
eli.friedman at gmail.com
Tue Jan 24 21:34:22 PST 2012
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.
(Expect to see some substantially faster progress on lambdas once I'm
done with this.)
-Eli
-------------- next part --------------
Index: include/clang/Sema/Sema.h
===================================================================
--- include/clang/Sema/Sema.h (revision 148922)
+++ include/clang/Sema/Sema.h (working copy)
@@ -2266,6 +2266,10 @@
void MarkDeclarationsReferencedInType(SourceLocation Loc, QualType T);
void MarkDeclarationsReferencedInExpr(Expr *E);
+ void MarkVarODRUsed(VarDecl *Var, SourceLocation Loc);
+ void MarkUseAsLValue(Expr *E);
+ 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/SemaCast.cpp
===================================================================
--- lib/Sema/SemaCast.cpp (revision 148922)
+++ lib/Sema/SemaCast.cpp (working copy)
@@ -533,6 +533,9 @@
if (SrcExpr.isInvalid()) // if conversion failed, don't report another error
return;
+ if (ValueKind == VK_LValue)
+ Self.MarkUseAsLValue(SrcExpr.get());
+
QualType OrigSrcType = SrcExpr.get()->getType();
QualType DestType = Self.Context.getCanonicalType(this->DestType);
@@ -667,6 +670,9 @@
if (SrcExpr.isInvalid()) // if conversion failed, don't report another error
return;
+ if (ValueKind == VK_LValue)
+ Self.MarkUseAsLValue(SrcExpr.get());
+
unsigned msg = diag::err_bad_cxx_cast_generic;
if (TryConstCast(Self, SrcExpr.get(), DestType, /*CStyle*/false, msg) != TC_Success
&& msg != 0)
@@ -687,6 +693,9 @@
if (SrcExpr.isInvalid()) // if conversion failed, don't report another error
return;
+ if (ValueKind == VK_LValue)
+ Self.MarkUseAsLValue(SrcExpr.get());
+
unsigned msg = diag::err_bad_cxx_cast_generic;
TryCastResult tcr =
TryReinterpretCast(Self, SrcExpr, DestType,
@@ -747,6 +756,12 @@
return;
}
+ // FIXME: This isn't quite right: given
+ // "const int x = 10; return static_cast<const long&>(x);",
+ // x isn't used as an lvalue.
+ if (ValueKind == VK_LValue)
+ Self.MarkUseAsLValue(SrcExpr.get());
+
unsigned msg = diag::err_bad_cxx_cast_generic;
TryCastResult tcr
= TryStaticCast(Self, SrcExpr, DestType, Sema::CCK_OtherCast, OpRange, msg,
@@ -1793,6 +1808,11 @@
return;
}
+ // FIXME: This isn't quite right: given
+ // "const int x = 10; return (const long&)x;", x isn't used as an lvalue.
+ if (ValueKind == VK_LValue)
+ Self.MarkUseAsLValue(SrcExpr.get());
+
// AltiVec vector initialization with a single literal.
if (const VectorType *vecTy = DestType->getAs<VectorType>())
if (vecTy->getVectorKind() == VectorType::AltiVecVector
Index: lib/Sema/SemaExprCXX.cpp
===================================================================
--- lib/Sema/SemaExprCXX.cpp (revision 148922)
+++ lib/Sema/SemaExprCXX.cpp (working copy)
@@ -3777,6 +3777,8 @@
if (LHS.get()->getObjectKind() == OK_BitField ||
RHS.get()->getObjectKind() == OK_BitField)
OK = OK_BitField;
+ MarkUseAsLValue(LHS.get());
+ MarkUseAsLValue(RHS.get());
return LTy;
}
@@ -4390,6 +4392,9 @@
}
}
+ if (Base->isGLValue())
+ S.MarkUseAsLValue(Base);
+
return false;
}
@@ -4716,6 +4721,8 @@
return Owned(E);
}
+ MarkUseAsLValue(E);
+
// Otherwise, this rule does not apply in C++, at least not for the moment.
if (getLangOptions().CPlusPlus) return Owned(E);
Index: lib/Sema/SemaInit.cpp
===================================================================
--- lib/Sema/SemaInit.cpp (revision 148922)
+++ lib/Sema/SemaInit.cpp (working copy)
@@ -4785,7 +4785,7 @@
return ExprError();
}
- // Reference binding does not have any corresponding ASTs.
+ S.MarkUseAsLValue(CurInit.get());
// Check exception specifications
if (S.CheckExceptionSpecCompatibility(CurInit.get(), DestType))
Index: lib/Sema/SemaTemplate.cpp
===================================================================
--- lib/Sema/SemaTemplate.cpp (revision 148922)
+++ 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 148922)
+++ lib/Sema/SemaStmt.cpp (working copy)
@@ -2034,11 +2034,13 @@
///
/// This method checks to see if the argument is an acceptable l-value and
/// returns false if it is a case we can handle.
-static bool CheckAsmLValue(const Expr *E, Sema &S) {
+static bool CheckAsmLValue(Expr *E, Sema &S) {
// Type dependent expressions will be checked during instantiation.
if (E->isTypeDependent())
return false;
+ S.MarkUseAsLValue(E);
+
if (E->isLValue())
return false; // Cool, this is an lvalue.
Index: lib/Sema/SemaExpr.cpp
===================================================================
--- lib/Sema/SemaExpr.cpp (revision 148922)
+++ lib/Sema/SemaExpr.cpp (working copy)
@@ -404,6 +404,8 @@
if (T.hasQualifiers())
T = T.getUnqualifiedType();
+ MarkUseAsRValue(E);
+
ExprResult Res = Owned(ImplicitCastExpr::Create(Context, T, CK_LValueToRValue,
E, 0, VK_RValue));
@@ -5101,6 +5103,8 @@
if (commonRes.isInvalid())
return ExprError();
commonExpr = commonRes.take();
+ } else {
+ MarkUseAsLValue(commonExpr);
}
opaqueValue = new (Context) OpaqueValueExpr(commonExpr->getExprLoc(),
@@ -7118,6 +7122,8 @@
/// CheckForModifiableLvalue - Verify that E is a modifiable lvalue. If not,
/// emit an error and return true. If so, return false.
static bool CheckForModifiableLvalue(Expr *E, SourceLocation Loc, Sema &S) {
+ S.MarkUseAsLValue(E);
+
SourceLocation OrigLoc = Loc;
Expr::isModifiableLvalueResult IsLV = E->isModifiableLvalue(S.Context,
&Loc);
@@ -7333,6 +7339,8 @@
if (!RHS.get()->getType()->isVoidType())
S.RequireCompleteType(Loc, RHS.get()->getType(),
diag::err_incomplete_type);
+ } else if (RHS.get()->isGLValue()) {
+ S.MarkUseAsLValue(RHS.get());
}
return RHS.get()->getType();
@@ -7640,6 +7648,8 @@
S.Diag(OpLoc, diag::ext_typecheck_addrof_void) << op->getSourceRange();
}
+ S.MarkUseAsLValue(op);
+
// If the operand has type "type", the result has type "pointer to type".
if (op->getType()->isObjCObjectType())
return S.Context.getObjCObjectPointerType(op->getType());
@@ -8306,8 +8316,10 @@
// _Real and _Imag map ordinary l-values into ordinary l-values.
if (Input.isInvalid()) return ExprError();
if (Input.get()->getValueKind() != VK_RValue &&
- Input.get()->getObjectKind() == OK_Ordinary)
+ Input.get()->getObjectKind() == OK_Ordinary) {
+ MarkUseAsLValue(Input.get());
VK = Input.get()->getValueKind();
+ }
break;
case UO_Extension:
resultType = Input.get()->getType();
@@ -9455,6 +9467,33 @@
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;
+ // FIXME: Need to extend MarkUseAsLValue to misc vector expressions.
+ if (T->isVectorType())
+ 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
@@ -9615,23 +9654,70 @@
}
}
- // 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);
}
+ return;
+ }
+}
- D->setUsed(true);
+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);
}
+Expr *GetMarkSubExpr(Expr *E) {
+ E = E->IgnoreParens();
+ // FIXME: This should be for ImplicitCastExprs only, but it's impossible
+ // to distinguish an __unknown_anytype cast from a real cast.
+ CastExpr *CE = dyn_cast<CastExpr>(E);
+ if (CE && CE->getCastKind() == CK_NoOp)
+ return GetMarkSubExpr(CE->getSubExpr());
+ return E;
+}
+
+void Sema::MarkUseAsRValue(Expr *E) {
+ E = GetMarkSubExpr(E);
+ if (OpaqueValueExpr *OVE = dyn_cast<OpaqueValueExpr>(E)) {
+ // FIXME: Is this handling correct?
+ if (OVE->getSourceExpr())
+ E = GetMarkSubExpr(OVE->getSourceExpr());
+ }
+ if (DeclRefExpr *DRE = dyn_cast<DeclRefExpr>(E->IgnoreParens())) {
+ VarDecl *Var = dyn_cast<VarDecl>(DRE->getDecl());
+ if (Var && DelayVarODRUsedForType(Var->getType()) &&
+ !DRE->isEvaluatable(Context))
+ MarkVarODRUsed(Var, DRE->getLocation());
+ }
+}
+
+void Sema::MarkUseAsLValue(Expr *E) {
+ E = GetMarkSubExpr(E);
+ if (OpaqueValueExpr *OVE = dyn_cast<OpaqueValueExpr>(E)) {
+ // FIXME: Is this handling correct?
+ if (OVE->getSourceExpr())
+ E = GetMarkSubExpr(OVE->getSourceExpr());
+ }
+ if (DeclRefExpr *DRE = dyn_cast<DeclRefExpr>(E)) {
+ VarDecl *Var = dyn_cast<VarDecl>(DRE->getDecl());
+ if (Var && DelayVarODRUsedForType(Var->getType()))
+ MarkVarODRUsed(Var, DRE->getLocation());
+ }
+}
+
namespace {
// Mark all of the declarations referenced
// FIXME: Not fully implemented yet! We need to have a better understanding
@@ -10080,6 +10166,7 @@
DestType = Ptr->getPointeeType();
ExprResult SubResult = Visit(E->getSubExpr());
if (SubResult.isInvalid()) return ExprError();
+ S.MarkUseAsLValue(SubResult.get());
E->setSubExpr(SubResult.take());
return E;
}
More information about the cfe-commits
mailing list