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

[Michael Kruse via Phabricator via cfe-commits]

Meinersbur added a comment.

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.

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.

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.

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.



================
Comment at: clang/include/clang/AST/StmtOpenMP.h:147
+
+  Stmt *getRawStmt() {
+    assert(HasAssociatedStmt &&
----------------
This looks like the equivalent to `ignoreCaptures` from D76342. Could you document it what it does differently from IgnoreContainers/getCapturedStmt/getInnermostCapturedStmt/getBody/getStructuredBlock?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D83261/new/

https://reviews.llvm.org/D83261