Change the AST generated for offsetof a bit so that it looks like a

normal expression, and change Evaluate and IRGen to evaluate it like a 
normal expression.  This simplifies the code significantly, and fixes 
PR3396.

llvm-svn: 65622

7 files changed, 35 insertions, 95 deletions

diff --git a/clang/include/clang/AST/Expr.h b/clang/include/clang/AST/Expr.h
index baf8c13279b..e7c97028526 100644
--- a/clang/include/clang/AST/Expr.h
+++ b/clang/include/clang/AST/Expr.h
@@ -656,8 +656,6 @@ public:
   }
   static bool classof(const UnaryOperator *) { return true; }
   
-  int64_t evaluateOffsetOf(ASTContext& C) const;
-  
   // Iterators
   virtual child_iterator child_begin();
   virtual child_iterator child_end();
diff --git a/clang/lib/AST/Expr.cpp b/clang/lib/AST/Expr.cpp
index e30925970db..d0d14f2b7af 100644
--- a/clang/lib/AST/Expr.cpp
+++ b/clang/lib/AST/Expr.cpp
@@ -1331,49 +1331,6 @@ bool ChooseExpr::isConditionTrue(ASTContext &C) const {
   return getCond()->getIntegerConstantExprValue(C) != 0;
 }
 
-static int64_t evaluateOffsetOf(ASTContext& C, const Expr *E) {
-  if (const MemberExpr *ME = dyn_cast<MemberExpr>(E)) {
-    QualType Ty = ME->getBase()->getType();
-    
-    RecordDecl *RD = Ty->getAsRecordType()->getDecl();
-    const ASTRecordLayout &RL = C.getASTRecordLayout(RD);
-    if (FieldDecl *FD = dyn_cast<FieldDecl>(ME->getMemberDecl())) {
-      // FIXME: This is linear time. And the fact that we're indexing
-      // into the layout by position in the record means that we're
-      // either stuck numbering the fields in the AST or we have to keep
-      // the linear search (yuck and yuck).
-      unsigned i = 0;
-      for (RecordDecl::field_iterator Field = RD->field_begin(),
-                                   FieldEnd = RD->field_end();
-           Field != FieldEnd; (void)++Field, ++i) {
-        if (*Field == FD)
-          break;
-      }
-      
-      return RL.getFieldOffset(i) + evaluateOffsetOf(C, ME->getBase());
-    }
-  } else if (const ArraySubscriptExpr *ASE = dyn_cast<ArraySubscriptExpr>(E)) {
-    const Expr *Base = ASE->getBase();
-    
-    int64_t size = C.getTypeSize(ASE->getType());
-    size *= ASE->getIdx()->getIntegerConstantExprValue(C).getSExtValue();
-    
-    return size + evaluateOffsetOf(C, Base);
-  } else if (isa<CompoundLiteralExpr>(E))
-    return 0;  
-
-  assert(0 && "Unknown offsetof subexpression!");
-  return 0;
-}
-
-int64_t UnaryOperator::evaluateOffsetOf(ASTContext& C) const
-{
-  assert(Opc == OffsetOf && "Unary operator not offsetof!");
-  
-  unsigned CharSize = C.Target.getCharWidth();
-  return ::evaluateOffsetOf(C, cast<Expr>(Val)) / CharSize;
-}
-
 void SizeOfAlignOfExpr::Destroy(ASTContext& C) {
   // Override default behavior of traversing children. If this has a type
   // operand and the type is a variable-length array, the child iteration
diff --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp
index 4f55f8d7642..68818dbf7b2 100644
--- a/clang/lib/AST/ExprConstant.cpp
+++ b/clang/lib/AST/ExprConstant.cpp
@@ -1064,8 +1064,16 @@ bool IntExprEvaluator::VisitSizeOfAlignOfExpr(const SizeOfAlignOfExpr *E) {
 bool IntExprEvaluator::VisitUnaryOperator(const UnaryOperator *E) {
   // Special case unary operators that do not need their subexpression
   // evaluated.  offsetof/sizeof/alignof are all special.
-  if (E->isOffsetOfOp())
-    return Success(E->evaluateOffsetOf(Info.Ctx), E);
+  if (E->isOffsetOfOp()) {
+    // The AST for offsetof is defined in such a way that we can just
+    // directly Evaluate it as an l-value.
+    APValue LV;
+    if (!EvaluateLValue(E->getSubExpr(), LV, Info))
+      return false;
+    if (LV.getLValueBase())
+      return false;
+    return Success(LV.getLValueOffset(), E);
+  }
 
   if (E->getOpcode() == UnaryOperator::LNot) {
     // LNot's operand isn't necessarily an integer, so we handle it specially.
diff --git a/clang/lib/AST/StmtPrinter.cpp b/clang/lib/AST/StmtPrinter.cpp
index 4468c79e91b..7e25b465b12 100644
--- a/clang/lib/AST/StmtPrinter.cpp
+++ b/clang/lib/AST/StmtPrinter.cpp
@@ -724,7 +724,7 @@ void StmtPrinter::VisitUnaryOperator(UnaryOperator *Node) {
 }
 
 bool StmtPrinter::PrintOffsetOfDesignator(Expr *E) {
-  if (isa<CompoundLiteralExpr>(E)) {
+  if (isa<UnaryOperator>(E)) {
     // Base case, print the type and comma.
     OS << E->getType().getAsString() << ", ";
     return true;
diff --git a/clang/lib/CodeGen/CGExprScalar.cpp b/clang/lib/CodeGen/CGExprScalar.cpp
index 020c21f5cbd..c65c0816e2b 100644
--- a/clang/lib/CodeGen/CGExprScalar.cpp
+++ b/clang/lib/CodeGen/CGExprScalar.cpp
@@ -755,47 +755,9 @@ Value *ScalarExprEmitter::VisitUnaryImag(const UnaryOperator *E) {
 
 Value *ScalarExprEmitter::VisitUnaryOffsetOf(const UnaryOperator *E)
 {
-  const Expr* SubExpr = E->getSubExpr();
+  Value* ResultAsPtr = EmitLValue(E->getSubExpr()).getAddress();
   const llvm::Type* ResultType = ConvertType(E->getType());
-  llvm::Value* Result = llvm::Constant::getNullValue(ResultType);
-  while (!isa<CompoundLiteralExpr>(SubExpr)) {
-    if (const MemberExpr *ME = dyn_cast<MemberExpr>(SubExpr)) {
-      SubExpr = ME->getBase();
-      QualType Ty = SubExpr->getType();
-
-      RecordDecl *RD = Ty->getAsRecordType()->getDecl();
-      const ASTRecordLayout &RL = CGF.getContext().getASTRecordLayout(RD);
-      FieldDecl *FD = cast<FieldDecl>(ME->getMemberDecl());
-
-      // FIXME: This is linear time. And the fact that we're indexing
-      // into the layout by position in the record means that we're
-      // either stuck numbering the fields in the AST or we have to keep
-      // the linear search (yuck and yuck).
-      unsigned i = 0;
-      for (RecordDecl::field_iterator Field = RD->field_begin(),
-                                   FieldEnd = RD->field_end();
-           Field != FieldEnd; (void)++Field, ++i) {
-        if (*Field == FD)
-          break;
-      }
-
-      llvm::Value* Offset =
-          llvm::ConstantInt::get(ResultType, RL.getFieldOffset(i) / 8);
-      Result = Builder.CreateAdd(Result, Offset);
-    } else if (const ArraySubscriptExpr *ASE = dyn_cast<ArraySubscriptExpr>(SubExpr)) {
-      SubExpr = ASE->getBase();
-      int64_t size = CGF.getContext().getTypeSize(ASE->getType()) / 8;
-      llvm::Value* ElemSize = llvm::ConstantInt::get(ResultType, size);
-      llvm::Value* ElemIndex = CGF.EmitScalarExpr(ASE->getIdx());
-      bool IndexSigned = ASE->getIdx()->getType()->isSignedIntegerType();
-      ElemIndex = Builder.CreateIntCast(ElemIndex, ResultType, IndexSigned);
-      llvm::Value* Offset = Builder.CreateMul(ElemSize, ElemIndex);
-      Result = Builder.CreateAdd(Result, Offset);
-    } else {
-      assert(0 && "This should be impossible!");
-    }
-  }
-  return Result;
+  return Builder.CreatePtrToInt(ResultAsPtr, ResultType, "offsetof");
 }
 
 //===----------------------------------------------------------------------===//
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index 29d5f59de91..43cbe98d875 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -4270,16 +4270,17 @@ Sema::ExprResult Sema::ActOnBuiltinOffsetOf(Scope *S,
   if (!Dependent && !ArgTy->isRecordType())
     return Diag(TypeLoc, diag::err_offsetof_record_type) << ArgTy;
 
-  // Otherwise, create a compound literal expression as the base, and
-  // iteratively process the offsetof designators.
-  InitListExpr *IList =
-      new (Context) InitListExpr(SourceLocation(), 0, 0, SourceLocation());
-  IList->setType(ArgTy);
-  Expr *Res =
-      new (Context) CompoundLiteralExpr(SourceLocation(), ArgTy, IList, false);
+  // Otherwise, create a null pointer as the base, and iteratively process
+  // the offsetof designators.
+  QualType ArgTyPtr = Context.getPointerType(ArgTy);
+  Expr* Res = new (Context) ImplicitValueInitExpr(ArgTyPtr);
+  Res = new (Context) UnaryOperator(Res, UnaryOperator::Deref, 
+                                    ArgTy, SourceLocation());
 
   // offsetof with non-identifier designators (e.g. "offsetof(x, a.b[c])") are a
   // GCC extension, diagnose them.
+  // FIXME: This diagnostic isn't actually visible because the location is in
+  // a system header!
   if (NumComponents != 1)
     Diag(BuiltinLoc, diag::ext_offsetof_extended_field_designator)
       << SourceRange(CompPtr[1].LocStart, CompPtr[NumComponents-1].LocEnd);
@@ -4300,6 +4301,10 @@ Sema::ExprResult Sema::ActOnBuiltinOffsetOf(Scope *S,
 
         // FIXME: C++: Verify that operator[] isn't overloaded.
 
+        // Promote the array so it looks more like a normal array subscript
+        // expression.
+        DefaultFunctionArrayConversion(Res);
+
         // C99 6.5.2.1p1
         Expr *Idx = static_cast<Expr*>(OC.U.E);
         if (!Idx->isTypeDependent() && !Idx->getType()->isIntegerType())
diff --git a/clang/test/Sema/offsetof.c b/clang/test/Sema/offsetof.c
index 5fd2a30eb7e..0d33b775519 100644
--- a/clang/test/Sema/offsetof.c
+++ b/clang/test/Sema/offsetof.c
@@ -35,3 +35,13 @@ int v2 = (int)(&((struct s2 *) 0)->a) == 0 ? 0 : f();
 
 struct s3 { int a; }; 
 int v3 = __builtin_offsetof(struct s3, a) == 0 ? 0 : f();
+
+// PR3396
+struct sockaddr_un {
+ unsigned char sun_len;
+ char sun_path[104];
+};
+int a(int len) {
+int a[__builtin_offsetof(struct sockaddr_un, sun_path[len+1])];
+}
+