george.burgess.iv added inline comments.

================
Comment at: lib/AST/ExprConstant.cpp:4763
@@ +4762,3 @@
+  /// arithmetic.
+  bool UseStrictCastingRules;
+
----------------
rsmith wrote:
> This should be handled as an `EvaluationMode`.
Works for me.

================
Comment at: lib/AST/ExprConstant.cpp:4879-4907
@@ +4878,31 @@
+  // We need to adjust the array index we're handing off in that case.
+  QualType ArrayTy = Result.Designator.MostDerivedType;
+  if (!ArrayTy.isNull() && !ArrayTy->isIncompleteType() && ArrayTy != Pointee) 
{
+    CharUnits PointeeEltSize;
+    if (!HandleSizeof(Info, PExp->getExprLoc(), Pointee, PointeeEltSize))
+      return false;
+
+    CharUnits ArrayEltSize;
+    if (!HandleSizeof(Info, PExp->getExprLoc(), ArrayTy, ArrayEltSize))
+      return false;
+
+    // Additionally, because LValuePathEntry wasn't made to really handle byte
+    // offsets, we need to keep the LValue Offset up-to-date (so casting e.g.
+    // BaseToDerived works as intended), but we also need to lie a bit about 
the
+    // array index.
+    //
+    // Example:
+    // (char*)Foo.Bar[1] + 2 // (assuming sizeof(Foo.Bar[1]) > 2)
+    // Will have an index of 1 and an offset of offsetof(Foo.Bar[1]) + 2.
+    if (PointeeEltSize != ArrayEltSize) {
+      CharUnits ByteOffset = IndexOffset * PointeeEltSize + AddedOffset;
+      CharUnits Rem = CharUnits::fromQuantity(ByteOffset % ArrayEltSize);
+      Result.Offset += Rem - AddedOffset;
+      AddedOffset = Rem;
+      IndexOffset = ByteOffset / ArrayEltSize;
+    }
+
+    // HandleLValueArrayAdjustment takes the type of the array
+    Pointee = ArrayTy;
+  }
+
----------------
rsmith wrote:
> We should not do this except in your special new mode, and I'm not entirely 
> convinced we should support this case at all. Just discarding the designator 
> (as we would do anyway in constant-folding mode) seems like an acceptable 
> response here. We really don't know what subobject is being referenced any 
> more.
We need some way to support `BOS((char*)&Foo.Bar + 1, N)`, so this needs to go 
somewhere. I originally had it in its own method that was super similar to this 
one, and the duplication made me sad. Will re-duplicate and see how that looks.

FYI, if you'll look at the diff at L194 in p2-0x.cpp, we have an interesting 
feature in the non-patched impl where `B *b = (B*)((A*)&b + 1)` is at an offset 
of sizeof(A), but Designator says it's at b[1]. I assumed this behavior is a 
bug, and this kinda-fixes it. If it's intended behavior, I'll absolutely find a 
better way to handle this.

================
Comment at: lib/AST/ExprConstant.cpp:6355-6356
@@ +6354,4 @@
+bool OutermostMemberEvaluator::VisitDeclRefExpr(const DeclRefExpr *E) {
+  Result = E->getType();
+  return true;
+}
----------------
rsmith wrote:
> I think handling this through the normal pointer evaluator, but with a 
> different `EvaluationMode`, is a better approach. (We need some 
> representation for an `LValueBase` to represent "unknown base", but 
> everything else should fall through pretty naturally.)
Sounds great to me -- I thought another visitor seemed heavyweight, but didn't 
consider adding another `EvaluationMode`. Will look into swapping approaches.


http://reviews.llvm.org/D12169



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to