[cfe-commits] r59420 - /cfe/trunk/lib/AST/ExprConstant.cpp

[Eli Friedman]

On Sun, Nov 16, 2008 at 11:01 AM, Anders Carlsson <andersca at mac.com> wrote:
> Author: andersca
> Date: Sun Nov 16 13:01:22 2008
> New Revision: 59420
>
> URL: http://llvm.org/viewvc/llvm-project?rev=59420&view=rev
> Log:
> More work on the constant evaluator. Eli, it would be great if you could have a look at this.

Okay... it looks generally fine.  More comments below:

> +APValue LValueExprEvaluator::VisitArraySubscriptExpr(ArraySubscriptExpr *E)
> +{
> +  APValue Result;
> +
> +  if (!EvaluatePointer(E->getBase(), Result, Info))
> +    return APValue();

I don't think this can actually happen, but it might be a good idea to
double-check that the base is in fact a pointer.

> +  APSInt Index;
> +  if (!EvaluateInteger(E->getIdx(), Index, Info))
> +    return APValue();
> +
> +  uint64_t ElementSize = Info.Ctx.getTypeSize(E->getType()) / 8;
> +
> +  uint64_t Offset = Index.getSExtValue() * ElementSize;

This could potentially crash once we support integers larger than 64
bits.  Also, this needs to be aware of the sign; we don't want to
sign-extend an unsigned short.

> +  bool VisitCXXBoolLiteralExpr(const CXXBoolLiteralExpr *E) {
> +    Result = E->getValue();
> +    Result.setIsUnsigned(E->getType()->isUnsignedIntegerType());
> +    return true;
> +  }

I think it would be a good idea to set the width of the result explicitly.

> +  if (E->getOpcode() == BinaryOperator::Sub) {
> +    if (LHSTy->isPointerType()) {
> +      if (RHSTy->isIntegralType()) {
> +        // pointer - int.
> +        // FIXME: Implement.

It should be impossible to land here for that case: pointer-int
doesn't return an int.

> +      // FIXME: Is this correct? What if only one of the operands has a base?
> +      if (LHSValue.getLValueBase() || RHSValue.getLValueBase())
> +        return false;

This is conservatively correct; a more aggressive constraint would be
that the bases are identical.

> +      const QualType Type = E->getLHS()->getType();
> +      const QualType ElementType = Type->getAsPointerType()->getPointeeType();
> +
> +      uint64_t D = LHSValue.getLValueOffset() - RHSValue.getLValueOffset();
> +      D /= Info.Ctx.getTypeSize(ElementType) / 8;

The result here isn't necessarily positive; an unsigned divide will
give an incorrect result in such cases.

> +      Result = D;
> +      Result.zextOrTrunc(getIntTypeSizeInBits(E->getType()));
> +      Result.setIsUnsigned(E->getType()->isUnsignedIntegerType());

You need to set the width of the result first; otherwise, you might
incorrectly zext the result.

-Eli