[PCH] Make sure that the SourceExpr of a OpaqueValueExpr is always initialized

when deserialized, fixing random crashes in libclang.

Also simplifies how OpaqueValueExprs are [de]serialized.
The reader/writer automatically retains pointer equality of sub-statements (when a
statement node is referenced in multiple nodes), so no need to manually handle it.

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@145752 91177308-0d34-0410-b5e6-96231b3b80d8

2 files changed, 4 insertions, 27 deletions

diff --git a/lib/Serialization/ASTReaderStmt.cpp b/lib/Serialization/ASTReaderStmt.cpp
index 91a6e1922f..65081c3560 100644
--- a/lib/Serialization/ASTReaderStmt.cpp
+++ b/lib/Serialization/ASTReaderStmt.cpp
@@ -568,8 +568,6 @@ ASTStmtReader::VisitBinaryConditionalOperator(BinaryConditionalOperator *E) {
   E->SubExprs[BinaryConditionalOperator::RHS] = Reader.ReadSubExpr();
   E->QuestionLoc = ReadSourceLocation(Record, Idx);
   E->ColonLoc = ReadSourceLocation(Record, Idx);
-
-  E->getOpaqueValue()->setSourceExpr(E->getCommon());
 }
 
 void ASTStmtReader::VisitImplicitCastExpr(ImplicitCastExpr *E) {
@@ -787,8 +785,6 @@ void ASTStmtReader::VisitPseudoObjectExpr(PseudoObjectExpr *E) {
   // Read all the semantic expressions.
   for (unsigned i = 0; i != numSemanticExprs; ++i) {
     Expr *subExpr = Reader.ReadSubExpr();
-    if (isa<OpaqueValueExpr>(subExpr))
-      cast<OpaqueValueExpr>(subExpr)->setSourceExpr(Reader.ReadSubExpr());
     E->getSubExprsBuffer()[i+1] = subExpr;
   }
 }
@@ -1376,7 +1372,7 @@ void ASTStmtReader::VisitMaterializeTemporaryExpr(MaterializeTemporaryExpr *E) {
 
 void ASTStmtReader::VisitOpaqueValueExpr(OpaqueValueExpr *E) {
   VisitExpr(E);
-  Idx++; // skip ID
+  E->SourceExpr = Reader.ReadSubExpr();
   E->Loc = ReadSourceLocation(Record, Idx);
 }
 
@@ -2055,20 +2051,9 @@ Stmt *ASTReader::ReadStmtFromStream(ModuleFile &F) {
       S = new (Context) MaterializeTemporaryExpr(Empty);
       break;
         
-    case EXPR_OPAQUE_VALUE: {
-      unsigned key = Record[ASTStmtReader::NumExprFields];
-      OpaqueValueExpr *&expr = OpaqueValueExprs[key];
-
-      // If we already have an entry for this opaque value expression,
-      // don't bother reading it again.
-      if (expr) {
-        StmtStack.push_back(expr);
-        continue;
-      }
-
-      S = expr = new (Context) OpaqueValueExpr(Empty);
+    case EXPR_OPAQUE_VALUE:
+      S = new (Context) OpaqueValueExpr(Empty);
       break;
-    }
 
     case EXPR_CUDA_KERNEL_CALL:
       S = new (Context) CUDAKernelCallExpr(Context, Empty);
diff --git a/lib/Serialization/ASTWriterStmt.cpp b/lib/Serialization/ASTWriterStmt.cpp
index e3311e9d90..9f976f4a10 100644
--- a/lib/Serialization/ASTWriterStmt.cpp
+++ b/lib/Serialization/ASTWriterStmt.cpp
@@ -750,8 +750,6 @@ void ASTStmtWriter::VisitPseudoObjectExpr(PseudoObjectExpr *E) {
   for (PseudoObjectExpr::semantics_iterator
          i = E->semantics_begin(), e = E->semantics_end(); i != e; ++i) {
     Writer.AddStmt(*i);
-    if (OpaqueValueExpr *OVE = dyn_cast<OpaqueValueExpr>(*i))
-      Writer.AddStmt(OVE->getSourceExpr());
   }
 
   Code = serialization::EXPR_PSEUDO_OBJECT;
@@ -1381,7 +1379,7 @@ void ASTStmtWriter::VisitMaterializeTemporaryExpr(MaterializeTemporaryExpr *E) {
 
 void ASTStmtWriter::VisitOpaqueValueExpr(OpaqueValueExpr *E) {
   VisitExpr(E);
-  Record.push_back(Writer.getOpaqueValueID(E));
+  Writer.AddStmt(E->getSourceExpr());
   Writer.AddSourceLocation(E->getLocation(), Record);
   Code = serialization::EXPR_OPAQUE_VALUE;
 }
@@ -1468,12 +1466,6 @@ void ASTWriter::ClearSwitchCaseIDs() {
   SwitchCaseIDs.clear();
 }
 
-unsigned ASTWriter::getOpaqueValueID(OpaqueValueExpr *e) {
-  unsigned &entry = OpaqueValues[e];
-  if (!entry) entry = OpaqueValues.size();
-  return entry;
-}
-
 /// \brief Write the given substatement or subexpression to the
 /// bitstream.
 void ASTWriter::WriteSubStmt(Stmt *S,