ABataev added a comment. In D83261#2161217 <https://reviews.llvm.org/D83261#2161217>, @Meinersbur wrote:
> AFAICS this extract out the handling of subnodes of OMPExecutableDirectives > into the OMPChildren class which is made optional since `OMPChildren > *OMPExecutableDirectives::Data` can be nullptr. However, since it also stores > clauses, it seems that about every executable directive will need to have one > anyway. Hence I don't see how it makes the representation of some executable > directives more correct. 1. OMPChildren class uses standard TrailingObjects harness instead of manual calculation. 2. Child Stmt* based nodes are not related to the AsssociatedStmt* anymore and can exist independently. > OMPChildren also handles clauses for OMPExecutableDirectives but not for > declarative directives. Should handling of of clauses also be extracted into > into its own class? That would make (de-)serialization easier for those as > well. This class is only for executable directives. > There is no effect on D76342 <https://reviews.llvm.org/D76342> (except a > requiring a rebase), since the OMPTileDirective has children and thus does > not profit from the functionality of `OMPChildren` becoming optional. Since I > don't see a relation to D76342 <https://reviews.llvm.org/D76342>, more , I am > indifferent to whether this should be merged. There should be an additional patch, which, I hope, should simplify things for loop-based directives. > Trailing objects is a technique to ensure that all substmts are consecutive > in memory (so `StmtIterator` can iterator over them). For > OMPExeuctableDirectives, only the associated statement is returned by the > `StmtIterator`, i.e. all the children could be made regular class members > without the complication of computing the offset. I'd prefer that change over > OMPChildren. There are also used_children, which are used by the clang analyzer for, at least, use of uninitialized variables diagnostics. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83261/new/ https://reviews.llvm.org/D83261 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
