16 nov 2008 kl. 13.59 skrev Eli Friedman:
On Sun, Nov 16, 2008 at 11:01 AM, Anders Carlsson <[EMAIL PROTECTED]> 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.
Hmm, not sure what you mean. EvaluatePointer will return false if the base is not 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.
I tried many different examples and couldn't come up with one that would fail. Do you have a concrete example? :)
+ 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.
Good idea. Fixed.
+ 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.
Fixed!
+ // 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.
Hmm, OK.
+ 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.
Same here. I couldn't reproduce the error with an example.
+ 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.
Fixed! Thanks for the review Eli! Anders
smime.p7s
Description: S/MIME cryptographic signature
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
