riccibruno added a comment. In D86207#2252409 <https://reviews.llvm.org/D86207#2252409>, @aaronpuchert wrote:
> In D86207#2251802 <https://reviews.llvm.org/D86207#2251802>, @riccibruno > wrote: > >> Is my comment on the deserialization of `BindingDecl`s in >> https://reviews.llvm.org/D85613?id=284364 related to this change? > > Not sure. The `FIXME` comment on the code is correct, and it would be correct > after this change. But notice that `Decomp` is also not set when constructing > a `BindingDecl` from the parser: first we build the `BindingDecl`s in > `Sema::ActOnDecompositionDeclarator` (there `Decomp` remains unset), then > build the `DecompositionDecl` from that in `Sema::ActOnVariableDeclarator`. > The constructor of `DecompositionDecl` is then calling `setDecomposedDecl` on > all bindings, so `Decomp` is set as soon as that's finished. But the > `BindingDecl`s exist without `Decomp` for a while. I agree, but I think that the `BindingDecl` should still be considered to be under construction until the corresponding `DecompositionDecl` is constructed. Any use of the `BindingDecl` in this interval has to be mindful that the `BindingDecl` is not fully constructed. > In D85613#2210054 <https://reviews.llvm.org/D85613#2210054>, @riccibruno > wrote: > >> The expression for the binding (`Binding`) will be deserialized when >> visiting the `BindingDecl`. This expression when non-null will always (as >> far as I can tell) contain a reference to the decomposition declaration so >> the decomposition will be deserialized which will set `Decomp`. > > My impression is that `Decomp` is set by > `ASTDeclReader::VisitDecompositionDecl`, after reading the individual > `BindingDecl`s, not from deserializing the `Binding` expression. Otherwise > the `BDs[I]->setDecomposedDecl(DD);` above wouldn't be needed. But maybe I'm > misunderstanding you. > > Maybe you're saying it might be a problem if I read the `BindingDecls` first > and then they can't reference the `DecompositionDecl`? I'm not sure how I can > see the deserialized AST, when I use `-ast-dump` with `-include-pch` it > doesn't show the AST of the included header. But the generated code looks > fine for an example I tried out. My concern is: Is it possible to see a deserialized `BindingDecl` when the corresponding `DecompositionDecl` has not been deserialized (which will as you say set `BindingDecl::Decomp`)? I have not been able to construct an example where this is the case but I am not at all confident that this is impossible. (`-ast-dump-all` can be added to force deserialization) 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
