[PATCH] D59214: [clang][OpeMP] Model OpenMP structured-block in AST (PR40563)

Alexey Bataev via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Mar 12 10:56:49 PDT 2019

ABataev added inline comments.

Comment at: include/clang/AST/StmtOpenMP.h:335
+  llvm::Optional<Stmt *> getStructuredBlockImpl() const {
+    return const_cast<Stmt *>(getInnermostCapturedStmt()->getCapturedStmt());
lebedev.ri wrote:
> ABataev wrote:
> > No need to insert it into each class, just add:
> > ```
> > Stmt * OMPExecutableDirective::getStructuredBlock() const {
> >   if (!hasAssociatedStmt() || !getAssociatedStmt())
> >     return nullptr;
> >   if (auto *LD = dyn_cast<OMPLoopDirective>(this))
> >     return LD->getBody();
> >   return getInnermostCapturedStmt()->getCapturedStmt();
> > }
> > ```
> I absolutely can do that, you are sure that is the most future-proof state?
> In particular, i want to re-point-out that if it's implemented like this,
> in the base class, then the sub-class may(will) not even know about this function,
> and thus 'forget' to update it, should it not be giving the correct answer for
> that new specific OpenMP executable directive.
> You are sure it's better to implement it in the `OMPExecutableDirective` itself?
Yes, I'm sure. It is the universal solution and all future classes must be compatible with it. If they are not, then they are incorrect.

  rC Clang



More information about the cfe-commits mailing list