[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

5 files changed, 20 insertions, 36 deletions

diff --git a/include/clang/Serialization/ASTReader.h b/include/clang/Serialization/ASTReader.h
index 203abe403c..552d1a5bff 100644
--- a/include/clang/Serialization/ASTReader.h
+++ b/include/clang/Serialization/ASTReader.h
@@ -585,9 +585,6 @@ private:
   /// switch statement can refer to them.
   std::map<unsigned, SwitchCase *> SwitchCaseStmts;
 
-  /// \brief Mapping from opaque value IDs to OpaqueValueExprs.
-  std::map<unsigned, OpaqueValueExpr*> OpaqueValueExprs;
-
   /// \brief The number of stat() calls that hit/missed the stat
   /// cache.
   unsigned NumStatHits, NumStatMisses;
diff --git a/include/clang/Serialization/ASTWriter.h b/include/clang/Serialization/ASTWriter.h
index fd2b9ec77e..7dd018d9fe 100644
--- a/include/clang/Serialization/ASTWriter.h
+++ b/include/clang/Serialization/ASTWriter.h
@@ -321,9 +321,6 @@ private:
   /// \brief Mapping from SwitchCase statements to IDs.
   std::map<SwitchCase *, unsigned> SwitchCaseIDs;
 
-  /// \brief Mapping from OpaqueValueExpr expressions to IDs.
-  llvm::DenseMap<OpaqueValueExpr *, unsigned> OpaqueValues;
-
   /// \brief The number of statements written to the AST file.
   unsigned NumStatements;
 
@@ -643,9 +640,6 @@ public:
 
   void ClearSwitchCaseIDs();
 
-  /// \brief Retrieve the ID for the given opaque value expression.
-  unsigned getOpaqueValueID(OpaqueValueExpr *e);
-
   unsigned getDeclParmVarAbbrev() const { return DeclParmVarAbbrev; }
   unsigned getDeclRefExprAbbrev() const { return DeclRefExprAbbrev; }
   unsigned getCharacterLiteralAbbrev() const { return CharacterLiteralAbbrev; }
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,
diff --git a/test/Index/pch-opaque-value.cpp b/test/Index/pch-opaque-value.cpp
new file mode 100644
index 0000000000..ae6b6dc588
--- /dev/null
+++ b/test/Index/pch-opaque-value.cpp
@@ -0,0 +1,16 @@
+struct S {
+  S();
+  S(int);
+  S(const S &);
+  ~S();
+};
+
+void f() {
+  try {
+  } catch (S e) {
+  }
+}
+
+// RUN: c-index-test -write-pch %t.pch %s
+// RUN: c-index-test -test-load-tu-usrs %t.pch local | FileCheck %s
+// CHECK: pch-opaque-value.cpp c:pch-opaque-value.cpp@86@F@f#@e Extent=[10:12 - 10:15]