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

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added a comment.

I would suggest starting with the reworking of the existing implementation with StmtVisitor class and after that step-by-step extend the functionality of the visitor with handling other kinds а expressions.



================
Comment at: clang/lib/Sema/SemaOpenMP.cpp:15274-15276
+      const std::string TypeStr = PT->getCanonicalTypeInternal().getAsString();
+      size_t TypeDecorCnt = std::count_if(TypeStr.begin(), TypeStr.end(),
+                                 [](char c) { return c == '*' || c == '['; });
----------------
Again, this definitely should be reworked.


================
Comment at: clang/lib/Sema/SemaOpenMP.cpp:15293
+      //  TypeDecorCnt for B: 0
+      if (Depth <= TypeDecorCnt) {
+        RelevantExpr = cast<Expr>(DRE);
----------------
The check is really bad. If you want to handle something like `*(B+x)` better to treat, say, `B` as the base and `x` as the index and, thus, treat the whole expression as something like `B[x]`


================
Comment at: clang/lib/Sema/SemaOpenMP.cpp:15303
+  bool VisitMemberExpr(MemberExpr *ME) {
+    Expr *E = cast<Expr>(ME);
+    Expr *BaseE = ME->getBase()->IgnoreParenImpCasts();
----------------
Why do you need this?


================
Comment at: clang/lib/Sema/SemaOpenMP.cpp:15338
+        return false;
+      return Visit(E->IgnoreParenImpCasts());;
+    }
----------------
Do not call `IgnoreParenImpCasts()` here, just `Visit(E)`. Also, I assume, it may lead to infinite recursion.


================
Comment at: clang/lib/Sema/SemaOpenMP.cpp:15356
       }
+      return Visit(E->IgnoreParenImpCasts());;
+    }
----------------
Same here


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72811