[PATCH] D91373: [OpenMP5.0] Support more kinds of lvalues in map clauses

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.


================
Comment at: clang/lib/Sema/SemaOpenMP.cpp:16869
+    // Allow results of method calls to be mapped.
+    if (isa<CXXMethodDecl>(ME->getMemberDecl())) {
+      RelevantExpr = ME;
----------------
I don't think it should always return `true`. What about `map(s.foo)` where `foo` is a member function?


================
Comment at: clang/lib/Sema/SemaOpenMP.cpp:16939
+    // forward to the source expression.
+    return Visit(OVE->getSourceExpr()->IgnoreParenImpCasts());
+  }
----------------
Same, too general check, plus in some cases `OVE->getSourceExpr()` may return `nullptr`.


================
Comment at: clang/lib/Sema/SemaOpenMP.cpp:17072
   bool VisitUnaryOperator(UnaryOperator *UO) {
-    if (SemaRef.getLangOpts().OpenMP < 50 || !UO->isLValue() ||
-        UO->getOpcode() != UO_Deref) {
+    if (SemaRef.getLangOpts().OpenMP < 50 ||
+        (Components.empty() &&
----------------
What is this for?


================
Comment at: clang/lib/Sema/SemaOpenMP.cpp:17115-17124
+  bool VisitCallExpr(CallExpr *CE) {
+    if (SemaRef.getLangOpts().OpenMP < 50 ||
+        (Components.empty() && !CE->isLValue())) {
+      emitErrorMsg();
+      return false;
+    }
+    assert(!RelevantExpr && "RelevantExpr is expected to be nullptr");
----------------
```
int a;
int &foo() {return a;}

...
#pragma omp target map(foo())
 foo() = 0;
...

```
How is this supposed to work?


================
Comment at: clang/lib/Sema/SemaOpenMP.cpp:17125-17144
+  bool VisitCastExpr(CastExpr *CE) {
+    if (SemaRef.getLangOpts().OpenMP < 50 ||
+        (Components.empty() && !CE->isLValue())) {
+      emitErrorMsg();
+      return false;
+    }
+    assert(!RelevantExpr && "RelevantExpr is expected to be nullptr");
----------------
Same questions here, how's the actual mapping is supposed to work? Need some more detailed description. I don't think this is going to be easy to implement it directly. We map the addresses of the base declarations but in your cases there is just no base declarations. SO, you need to create one. To me, this should look like this:
```
#pragma omp target map(<lvalue>)
{
  <lvalue> = xxx;
  yyy = <lvalue>;
}
```
|
V
```
auto &mapped = <lvalue>;
#pragma omp target map(mapped)
{
  mapped = xxx;
  yyy = mapped;
}
```



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

https://reviews.llvm.org/D91373