[PATCH] D99537: [OPENMP51]Initial support for the dispatch directive

[Mike Rice via Phabricator via llvm-commits]

mikerice added inline comments.


================
Comment at: clang/lib/Sema/SemaOpenMP.cpp:9766-9768
+namespace {
+class DispatchStmtExprChecker
+    : public StmtVisitor<DispatchStmtExprChecker, bool> {
----------------
ABataev wrote:
> mikerice wrote:
> > ABataev wrote:
> > > Why `Expr->IgnoreParenImpCasts()` does not work here?
> > No reason not to use IgnoreParenImpCasts() instead of IgnoreParens() as far as I know.  That should eliminate the need for visiting MaterializeTemporaryExpr.  It unfortunately doesn't skip over CXXBindTemporaryExpr.  Or were you suggesting something else?
> Then just extend `IgnoreParenImpCasts` to support `MaterializeTemporaryExpr` and control it by the parameter.
CXXBindTemporaryExpr isn't the only reason to use the visitor.  It ends up handling the top level statements and ExprWithCleanups easily and is fairly understandable to me.  I went ahead and hand wrote the code to see what it would look like.  Turns out to be long chains of one line if statement like this:


```
static Expr *getDirectCallExpr(Expr *E) {
  E = E->IgnoreParenImpCasts();
  if (auto *BTE = dyn_cast<CXXBindTemporaryExpr>(E))
    E = BTE->getSubExpr();
  if (auto *CE = dyn_cast<CallExpr>(E))
    if (CE->getDirectCallee())
      return E;
  return nullptr;
}

  SourceLocation TargetCallLoc;

  if (!CurContext->isDependentContext()) {
    Expr *TargetCall = nullptr;

    auto *E = dyn_cast<Expr>(S);
    if (!E) {
      Diag(S->getBeginLoc(), diag::err_omp_dispatch_statement_call);
      return StmtError();
    }

    if (auto *EWC = dyn_cast<ExprWithCleanups>(S))
      E = EWC->getSubExpr();

    E = E->IgnoreParenImpCasts();

    if (auto *CastE = dyn_cast<CastExpr>(E))
      if (CastE->getCastKind() == CK_ToVoid)
        E = CastE->getSubExpr()->IgnoreParenImpCasts();

    if (auto *COCE = dyn_cast<CXXOperatorCallExpr>(E)) {
      if (COCE->getOperator() == OO_Equal) {
        if (Expr *TE = getDirectCallExpr(COCE->getArg(1)))
          TargetCall = TE;
        else if (COCE->getDirectCallee())
          TargetCall = COCE;
      }
    } else if (auto *BO = dyn_cast<BinaryOperator>(E)) {
      if (BO->getOpcode() == BO_Assign)
        if (Expr *TE = getDirectCallExpr(BO->getRHS()))
          TargetCall = TE;
    } else if (auto *CE = dyn_cast<CallExpr>(E)) {
      if (CE->getDirectCallee())
        TargetCall = E;
    }
    if (!TargetCall) {
      Diag(E->getBeginLoc(), diag::err_omp_dispatch_statement_call);
      return StmtError();
    }
    TargetCallLoc = TargetCall->getExprLoc();
  }

```
I didn't originally write the visitor code so I am not attached to it or anything.  Do you think this is better?  Is there some rule-of-thumb when to use visitors and when to write it out by hand?


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

https://reviews.llvm.org/D99537