[PATCH] D86207: (De-)serialize BindingDecls before DecompositionDecl

[Aaron Puchert via Phabricator via cfe-commits]

aaronpuchert created this revision.
aaronpuchert added a reviewer: rsmith.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
aaronpuchert requested review of this revision.

When parsing a C++17 binding declaration, we first create the
BindingDecls in Sema::ActOnDecompositionDeclarator, and then build the
DecompositionDecl in Sema::ActOnVariableDeclarator, so the contained
BindingDecls are never null. But when deserializing, we read the
DecompositionDecl first, do a few operations on it, and only later read
the BindingDecls.

One of those operations is potentially marking the DecompositionDecl as
invalid, and Decl::setInvalidDecl was propagating that to the
BindingDecls that don't yet exist, causing a crash when deserializing
invalid DecompositionDecls.

One way would be to change that function, but I believe this is better,
because the parsing code path is tested more thoroughly and sticking to
the same order for (de-)serialization makes the assumptions one would
derive from the parsing code apply here as well.

Fixes PR34960.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D86207

Files:
  clang/lib/Serialization/ASTReaderDecl.cpp
  clang/lib/Serialization/ASTWriterDecl.cpp
  clang/test/PCH/cxx1z-decomposition.cpp


Index: clang/test/PCH/cxx1z-decomposition.cpp
===================================================================
--- clang/test/PCH/cxx1z-decomposition.cpp
+++ clang/test/PCH/cxx1z-decomposition.cpp
@@ -2,11 +2,11 @@
 // RUN: %clang_cc1 -pedantic -std=c++1z -include %s -verify %s
 //
 // With PCH:
-// RUN: %clang_cc1 -pedantic -std=c++1z -emit-pch %s -o %t
-// RUN: %clang_cc1 -pedantic -std=c++1z -include-pch %t -verify %s
+// RUN: %clang_cc1 -pedantic -std=c++1z -emit-pch -fallow-pch-with-compiler-errors %s -o %t
+// RUN: %clang_cc1 -pedantic -std=c++1z -include-pch %t -fallow-pch-with-compiler-errors -verify %s
 
-// RUN: %clang_cc1 -pedantic -std=c++1z -emit-pch -fpch-instantiate-templates %s -o %t
-// RUN: %clang_cc1 -pedantic -std=c++1z -include-pch %t -verify %s
+// RUN: %clang_cc1 -pedantic -std=c++1z -emit-pch -fallow-pch-with-compiler-errors -fpch-instantiate-templates %s -o %t
+// RUN: %clang_cc1 -pedantic -std=c++1z -include-pch %t -fallow-pch-with-compiler-errors -verify %s
 
 #ifndef HEADER
 #define HEADER
@@ -22,6 +22,8 @@
   return a * 10 + b;
 }
 
+auto [noinit]; // expected-error{{decomposition declaration '[noinit]' requires an initializer}}
+
 #else
 
 int arr[2];
Index: clang/lib/Serialization/ASTWriterDecl.cpp
===================================================================
--- clang/lib/Serialization/ASTWriterDecl.cpp
+++ clang/lib/Serialization/ASTWriterDecl.cpp
@@ -1140,9 +1140,9 @@
   // Record the number of bindings first to simplify deserialization.
   Record.push_back(D->bindings().size());
 
-  VisitVarDecl(D);
   for (auto *B : D->bindings())
     Record.AddDeclRef(B);
+  VisitVarDecl(D);
   Code = serialization::DECL_DECOMPOSITION;
 }
 
Index: clang/lib/Serialization/ASTReaderDecl.cpp
===================================================================
--- clang/lib/Serialization/ASTReaderDecl.cpp
+++ clang/lib/Serialization/ASTReaderDecl.cpp
@@ -1495,12 +1495,12 @@
 }
 
 void ASTDeclReader::VisitDecompositionDecl(DecompositionDecl *DD) {
-  VisitVarDecl(DD);
   auto **BDs = DD->getTrailingObjects<BindingDecl *>();
   for (unsigned I = 0; I != DD->NumBindings; ++I) {
     BDs[I] = readDeclAs<BindingDecl>();
     BDs[I]->setDecomposedDecl(DD);
   }
+  VisitVarDecl(DD);
 }
 
 void ASTDeclReader::VisitBindingDecl(BindingDecl *BD) {


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D86207.286536.patch
Type: text/x-patch
Size: 2303 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20200819/535fa095/attachment-0001.bin>