[PATCH] D72811: [WIP][OPENMP5.0] allow lvalue for motion clause

[Chi Chun Chen via Phabricator via cfe-commits]

cchen marked an inline comment as done.
cchen added inline comments.


================
Comment at: clang/lib/Sema/SemaOpenMP.cpp:15442-15443
+      CurComponents.emplace_back(CurE, nullptr);
+    } else if (auto *CurE = dyn_cast<BinaryOperator>(E)) {
+      E = CurE->getLHS()->IgnoreParenImpCasts();
     } else {
----------------
ABataev wrote:
> cchen wrote:
> > cchen wrote:
> > > ABataev wrote:
> > > > Why just the LHS is analyzed? Also, what about support for other expressions, like casting, call, choose etc., which may result in lvalue?
> > > 1. LHS: I'll fix that
> > > 2. I'll add support for casting, call, etc
> > > 3. For "choose" are you referring to something like (a < b ? b : a)?
> > For the handling of BinaryOperator, I'm not sure why should we handle RHS since the possible source code I can imagine is `*(ptr+l)` or something like `(ptr+l)[3]`.
> `*(2+ptr)` is correct too. And, btw, `3[ptr+1]` too, especially in C. Moreover, something like `*(b+c)` is also allowed. That's why I said that better to avoid deep analysis of lvalues, there are too many combinations and better to switch to something basic.
But at least we need the base declaration for motion/map clause, right? And to achieve this, we need to peel the expression to get the DeclRefExpr.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72811