[PATCH] D83261: [OPENMP]Redesign of OMPExecutableDirective representation.

[Alexey Bataev via Phabricator via cfe-commits]

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