rsmith added inline comments.
================
Comment at: clang/lib/Serialization/ASTReaderDecl.cpp:585
D->setLocation(ThisDeclLoc);
D->setInvalidDecl(Record.readInt());
if (Record.readInt()) { // hasAttrs
----------------
The bug is here: we should not be calling `Decl::setInvalidDecl` here because
it has invariants, and the `Decl` is incomplete at this point.
================
Comment at: clang/lib/Serialization/ASTReaderDecl.cpp:1500
for (unsigned I = 0; I != DD->NumBindings; ++I) {
BDs[I] = readDeclAs<BindingDecl>();
BDs[I]->setDecomposedDecl(DD);
----------------
`BindingDecl` might not be fully initialized here: if we enter deserialization
to deserialize a `BindingDecl`, and then recurse into reading its binding
expression, and then deserialize the `DecompositionDecl`, we can land here
before we finish with the `BindingDecl`. If we called something that looked at
the `Binding` expression on the `BindingDecl`, that'd be a problem.
But the general idea is that deserialization should never invoke AST functions
with invariants (and generally should set state directly rather than using AST
member functions in order to avoid accidentally calling a function with an
invariant). So it shouldn't matter whether we deserialize the
`DecompositionDecl` or the `BindingDecl`s first.
================
Comment at: clang/lib/Serialization/ASTReaderDecl.cpp:1509
BD->Binding = Record.readExpr();
}
----------------
Hm, presumably `BindingDecl::Decomp` gets set here because `readExpr` always
reads an expression that involves the `DecompositionDecl`, which calls
`setDecomposedDecl`? That seems ... very subtle. If that's the intended way for
this to work, we should at least add a comment for that (or better, an assert
that `Decomp` got set), and `BindingDecl::Decomp` should be a regular pointer
not a `LazyDeclPtr`. (But even then, is this chain of reasoning guaranteed to
hold even for invalid declarations? Maybe we should be serializing the
`DecompositionDecl*` and setting `Decomp` here?)
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D86207/new/
https://reviews.llvm.org/D86207
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
