[cfe-commits] r147561 - in /cfe/trunk: include/clang/AST/APValue.h lib/AST/APValue.cpp lib/AST/Expr.cpp lib/AST/ExprConstant.cpp lib/CodeGen/CGExprConstant.cpp lib/Sema/SemaChecking.cpp test/CodeGenCXX/const-init.cpp

[Richard Smith]

On Wed, January 4, 2012 23:13, Eli Friedman wrote:
> Author: efriedma
> Date: Wed Jan  4 17:13:47 2012
> New Revision: 147561
>
>
> URL: http://llvm.org/viewvc/llvm-project?rev=147561&view=rev
> Log:
> Add an APValue representation for the difference between two address-of-label
> expressions.  Add support to Evaluate and CGExprConstant for
> generating/handling them.  Remove the special-case for such differences in
> Expr::isConstantInitializer.
>
>
> With that done, remove a bunch of buggy code from CGExprConstant for handling
> scalar expressions which is no longer necessary.

Awesome!

> Modified: cfe/trunk/lib/AST/APValue.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/APValue.cpp?rev=147561&
> r1=147560&r2=147561&view=diff
> =============================================================================
> =
> --- cfe/trunk/lib/AST/APValue.cpp (original)
> +++ cfe/trunk/lib/AST/APValue.cpp Wed Jan  4 17:13:47 2012
> @@ -472,6 +482,11 @@
> }
> Out << "0";
> return; +  case APValue::AddrLabelDiff:
> +    Out << "&&" << getAddrLabelDiffLHS()->getLabel()->getName();
> +    Out << " - ";
> +    Out << "&&" << getAddrLabelDiffRHS()->getLabel()->getName();
> +    return;

Please add a test for this to SemaCXX/constexpr-printing.cpp.

> Modified: cfe/trunk/lib/AST/ExprConstant.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ExprConstant.cpp?rev=14
> 7561&r1=147560&r2=147561&view=diff
> ==============================================================================
>  --- cfe/trunk/lib/AST/ExprConstant.cpp (original)
> +++ cfe/trunk/lib/AST/ExprConstant.cpp Wed Jan  4 17:13:47 2012
> @@ -210,6 +210,8 @@
> CCValue(const ValueDecl *D, bool IsDerivedMember,
> ArrayRef<const CXXRecordDecl*> Path) :
> APValue(D, IsDerivedMember, Path) {}
> +    CCValue(const AddrLabelExpr* LHSExpr, const AddrLabelExpr* RHSExpr) :
> +      APValue(LHSExpr, RHSExpr) {}
>
>
> CallStackFrame *getLValueFrame() const {
> assert(getKind() == LValue); @@ -856,6 +858,7 @@
> case APValue::Array: case APValue::Struct: case APValue::Union: +  case
> APValue::AddrLabelDiff:
> return false; }
>
>
> @@ -3997,6 +4000,23 @@
> // Reject differing bases from the normal codepath; we special-case
> // comparisons to null.
> if (!HasSameBase(LHSValue, RHSValue)) { +        if (E->getOpcode() == BO_Sub)
> {
> +          // Handle &&A - &&B.
> +          // FIXME: We're missing a check that both labels have the same
> +          // associated function; I'm not sure how to write this check.
> +          if (!LHSValue.Offset.isZero() || !RHSValue.Offset.isZero())
> +            return false;
> +          const Expr *LHSExpr = LHSValue.Base.dyn_cast<const Expr*>();
> +          const Expr *RHSExpr = LHSValue.Base.dyn_cast<const Expr*>();
> +          if (!LHSExpr || !RHSExpr)
> +            return false;
> +          const AddrLabelExpr *LHSAddrExpr =
> dyn_cast<AddrLabelExpr>(LHSExpr); +          const AddrLabelExpr *RHSAddrExpr
> = dyn_cast<AddrLabelExpr>(RHSExpr);
> +          if (!LHSAddrExpr || !RHSAddrExpr)
> +            return false;
> +          Result = CCValue(LHSAddrExpr, RHSAddrExpr);
> +          return true;
> +        }
> // Inequalities and subtractions between unrelated pointers have
> // unspecified or undefined behavior.
> if (!E->isEqualityOp()) @@ -4091,6 +4111,25 @@
> return true; }
>
>
> +  if (E->getOpcode() == BO_Sub && LHSVal.isLValue() && RHSVal.isLValue()) {
> +    // Handle (intptr_t)&&A - (intptr_t)&&B.
> +    // FIXME: We're missing a check that both labels have the same
> +    // associated function; I'm not sure how to write this check.

You can get the LabelDecl from the AddrLabelExpr, and the decl context of the
label decl should be the function, block or lambda containing the label.

> +    if (!LHSVal.getLValueOffset().isZero() ||
> +        !RHSVal.getLValueOffset().isZero())
> +      return false;
> +    const Expr *LHSExpr = LHSVal.getLValueBase().dyn_cast<const Expr*>();
> +    const Expr *RHSExpr = RHSVal.getLValueBase().dyn_cast<const Expr*>();
> +    if (!LHSExpr || !RHSExpr)
> +      return false;
> +    const AddrLabelExpr *LHSAddrExpr = dyn_cast<AddrLabelExpr>(LHSExpr);
> +    const AddrLabelExpr *RHSAddrExpr = dyn_cast<AddrLabelExpr>(RHSExpr);
> +    if (!LHSAddrExpr || !RHSAddrExpr)
> +      return false;
> +    Result = CCValue(LHSAddrExpr, RHSAddrExpr);
> +    return true;
> +  }

Please add a CCEDiag to both these cases (we don't want to treat this as an
ICE in C++11).

> Modified: cfe/trunk/lib/CodeGen/CGExprConstant.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGExprConstant.cpp?
> rev=147561&r1=147560&r2=147561&view=diff
> =============================================================================
> =
> @@ -1081,6 +1022,23 @@
> }
> return llvm::ConstantVector::get(Inits); }
> +    case APValue::AddrLabelDiff: {
> +      const AddrLabelExpr *LHSExpr = Result.Val.getAddrLabelDiffLHS();
> +      const AddrLabelExpr *RHSExpr = Result.Val.getAddrLabelDiffRHS();
> +      llvm::Constant *LHS = EmitConstantExpr(LHSExpr, LHSExpr->getType(),
> CGF);
> +      llvm::Constant *RHS = EmitConstantExpr(RHSExpr, RHSExpr->getType(),
> CGF);
> +
> +      // Compute difference
> +      llvm::Type *ResultType = getTypes().ConvertType(E->getType());
> +      LHS = llvm::ConstantExpr::getPtrToInt(LHS, IntPtrTy);
> +      RHS = llvm::ConstantExpr::getPtrToInt(RHS, IntPtrTy);
> +      llvm::Constant *AddrLabelDiff = llvm::ConstantExpr::getSub(LHS, RHS);
> +
> +      // LLVM os a bit sensitive about the exact format of the

Typo 'os'.

> +      // address-of-label difference; make sure to truncate after
> +      // the subtraction.
> +      return llvm::ConstantExpr::getTruncOrBitCast(AddrLabelDiff,
> ResultType);
> +    }

Thanks!
Richard