AtomicExpr: make ASTStmtReader a friend and remove setters. Also fix saving

of an uninitialized Stmt* in serialization of __atomic_init and add a test of
atomics serialization.

llvm-svn: 154448

5 files changed, 45 insertions, 51 deletions

diff --git a/clang/include/clang/AST/Expr.h b/clang/include/clang/AST/Expr.h
index e22c44d65a1..0db9195a278 100644
--- a/clang/include/clang/AST/Expr.h
+++ b/clang/include/clang/AST/Expr.h
@@ -4470,7 +4470,7 @@ public:
 
 /// AtomicExpr - Variadic atomic builtins: __atomic_exchange, __atomic_fetch_*,
 /// __atomic_load, __atomic_store, and __atomic_compare_exchange_*, for the
-/// similarly-named C++0x instructions.  All of these instructions take one
+/// similarly-named C++11 instructions.  All of these instructions take one
 /// primary pointer and at least one memory order.
 class AtomicExpr : public Expr {
 public:
@@ -4483,60 +4483,42 @@ private:
   SourceLocation BuiltinLoc, RParenLoc;
   AtomicOp Op;
 
+  friend class ASTStmtReader;
+
 public:
   AtomicExpr(SourceLocation BLoc, Expr **args, unsigned nexpr, QualType t,
              AtomicOp op, SourceLocation RP);
 
+  /// \brief Determine the number of arguments the specified atomic builtin
+  /// should have.
+  static unsigned getNumSubExprs(AtomicOp Op);
+
   /// \brief Build an empty AtomicExpr.
   explicit AtomicExpr(EmptyShell Empty) : Expr(AtomicExprClass, Empty) { }
 
   Expr *getPtr() const {
     return cast<Expr>(SubExprs[PTR]);
   }
-  void setPtr(Expr *E) {
-    SubExprs[PTR] = E;
-  }
   Expr *getOrder() const {
     return cast<Expr>(SubExprs[ORDER]);
   }
-  void setOrder(Expr *E) {
-    SubExprs[ORDER] = E;
-  }
   Expr *getVal1() const {
     if (Op == Init)
       return cast<Expr>(SubExprs[ORDER]);
     assert(NumSubExprs >= 3);
     return cast<Expr>(SubExprs[VAL1]);
   }
-  void setVal1(Expr *E) {
-    if (Op == Init) {
-      SubExprs[ORDER] = E;
-      return;
-    }
-    assert(NumSubExprs >= 3);
-    SubExprs[VAL1] = E;
-  }
   Expr *getOrderFail() const {
     assert(NumSubExprs == 5);
     return cast<Expr>(SubExprs[ORDER_FAIL]);
   }
-  void setOrderFail(Expr *E) {
-    assert(NumSubExprs == 5);
-    SubExprs[ORDER_FAIL] = E;
-  }
   Expr *getVal2() const {
     assert(NumSubExprs == 5);
     return cast<Expr>(SubExprs[VAL2]);
   }
-  void setVal2(Expr *E) {
-    assert(NumSubExprs == 5);
-    SubExprs[VAL2] = E;
-  }
 
   AtomicOp getOp() const { return Op; }
-  void setOp(AtomicOp op) { Op = op; }
   unsigned getNumSubExprs() { return NumSubExprs; }
-  void setNumSubExprs(unsigned num) { NumSubExprs = num; }
 
   Expr **getSubExprs() { return reinterpret_cast<Expr **>(SubExprs); }
 
@@ -4550,10 +4532,7 @@ public:
   }
 
   SourceLocation getBuiltinLoc() const { return BuiltinLoc; }
-  void setBuiltinLoc(SourceLocation L) { BuiltinLoc = L; }
-
   SourceLocation getRParenLoc() const { return RParenLoc; }
-  void setRParenLoc(SourceLocation L) { RParenLoc = L; }
 
   SourceRange getSourceRange() const LLVM_READONLY {
     return SourceRange(BuiltinLoc, RParenLoc);
diff --git a/clang/lib/AST/Expr.cpp b/clang/lib/AST/Expr.cpp
index 1947e61d12f..2bb79a0a4a1 100644
--- a/clang/lib/AST/Expr.cpp
+++ b/clang/lib/AST/Expr.cpp
@@ -3841,6 +3841,7 @@ AtomicExpr::AtomicExpr(SourceLocation BLoc, Expr **args, unsigned nexpr,
          false, false, false, false),
     NumSubExprs(nexpr), BuiltinLoc(BLoc), RParenLoc(RP), Op(op)
 {
+  assert(nexpr == getNumSubExprs(op) && "wrong number of subexpressions");
   for (unsigned i = 0; i < nexpr; i++) {
     if (args[i]->isTypeDependent())
       ExprBits.TypeDependent = true;
@@ -3854,3 +3855,23 @@ AtomicExpr::AtomicExpr(SourceLocation BLoc, Expr **args, unsigned nexpr,
     SubExprs[i] = args[i];
   }
 }
+
+unsigned AtomicExpr::getNumSubExprs(AtomicOp Op) {
+  switch (Op) {
+  case Init:
+  case Load:
+    return 2;
+  case Store:
+  case Xchg:
+  case Add:
+  case Sub:
+  case And:
+  case Or:
+  case Xor:
+    return 3;
+  case CmpXchgStrong:
+  case CmpXchgWeak:
+    return 5;
+  }
+  llvm_unreachable("unknown atomic op");
+}
diff --git a/clang/lib/Serialization/ASTReaderStmt.cpp b/clang/lib/Serialization/ASTReaderStmt.cpp
index 1c9817b955d..2eeb090af0d 100644
--- a/clang/lib/Serialization/ASTReaderStmt.cpp
+++ b/clang/lib/Serialization/ASTReaderStmt.cpp
@@ -790,21 +790,12 @@ void ASTStmtReader::VisitPseudoObjectExpr(PseudoObjectExpr *E) {
 
 void ASTStmtReader::VisitAtomicExpr(AtomicExpr *E) {
   VisitExpr(E);
-  E->setOp(AtomicExpr::AtomicOp(Record[Idx++]));
-  E->setPtr(Reader.ReadSubExpr());
-  E->setOrder(Reader.ReadSubExpr());
-  E->setNumSubExprs(2);
-  if (E->getOp() != AtomicExpr::Load) {
-    E->setVal1(Reader.ReadSubExpr());
-    E->setNumSubExprs(3);
-  }
-  if (E->isCmpXChg()) {
-    E->setOrderFail(Reader.ReadSubExpr());
-    E->setVal2(Reader.ReadSubExpr());
-    E->setNumSubExprs(5);
-  }
-  E->setBuiltinLoc(ReadSourceLocation(Record, Idx));
-  E->setRParenLoc(ReadSourceLocation(Record, Idx));
+  E->Op = AtomicExpr::AtomicOp(Record[Idx++]);
+  E->NumSubExprs = AtomicExpr::getNumSubExprs(E->Op);
+  for (unsigned I = 0; I != E->NumSubExprs; ++I)
+    E->SubExprs[I] = Reader.ReadSubExpr();
+  E->BuiltinLoc = ReadSourceLocation(Record, Idx);
+  E->RParenLoc = ReadSourceLocation(Record, Idx);
 }
 
 //===----------------------------------------------------------------------===//
diff --git a/clang/lib/Serialization/ASTWriterStmt.cpp b/clang/lib/Serialization/ASTWriterStmt.cpp
index eac7e1fb730..827caa026a4 100644
--- a/clang/lib/Serialization/ASTWriterStmt.cpp
+++ b/clang/lib/Serialization/ASTWriterStmt.cpp
@@ -751,14 +751,8 @@ void ASTStmtWriter::VisitPseudoObjectExpr(PseudoObjectExpr *E) {
 void ASTStmtWriter::VisitAtomicExpr(AtomicExpr *E) {
   VisitExpr(E);
   Record.push_back(E->getOp());
-  Writer.AddStmt(E->getPtr());
-  Writer.AddStmt(E->getOrder());
-  if (E->getOp() != AtomicExpr::Load)
-    Writer.AddStmt(E->getVal1());
-  if (E->isCmpXChg()) {
-    Writer.AddStmt(E->getOrderFail());
-    Writer.AddStmt(E->getVal2());
-  }
+  for (unsigned I = 0, N = E->getNumSubExprs(); I != N; ++I)
+    Writer.AddStmt(E->getSubExprs()[I]);
   Writer.AddSourceLocation(E->getBuiltinLoc(), Record);
   Writer.AddSourceLocation(E->getRParenLoc(), Record);
   Code = serialization::EXPR_ATOMIC;
diff --git a/clang/test/CodeGen/atomic-ops.c b/clang/test/CodeGen/atomic-ops.c
index 81406347735..2285cca7235 100644
--- a/clang/test/CodeGen/atomic-ops.c
+++ b/clang/test/CodeGen/atomic-ops.c
@@ -1,5 +1,12 @@
 // RUN: %clang_cc1 %s -emit-llvm -o - -triple=i686-apple-darwin9 | FileCheck %s
 
+// Also test serialization of atomic operations here, to avoid duplicating the
+// test.
+// RUN: %clang_cc1 %s -emit-pch -o %t -triple=i686-apple-darwin9
+// RUN: %clang_cc1 %s -include-pch %t -triple=i686-apple-darwin9 -emit-llvm -o - | FileCheck %s
+#ifndef ALREADY_INCLUDED
+#define ALREADY_INCLUDED
+
 // Basic IRGen tests for __atomic_*
 
 // FIXME: Need to implement __atomic_is_lock_free
@@ -117,3 +124,5 @@ int structAtomicCmpExchange() {
   return __atomic_compare_exchange_strong(&bigAtomic, &f, g, 5, 5);
   // CHECK: call zeroext i1 @__atomic_compare_exchange(i32 512, i8* bitcast (%struct.foo* @bigAtomic to i8*), 
 }
+
+#endif