[PATCH] D127803: Generate the capture for field when the field is used in openmp region with implicit default in the member function.

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.


================
Comment at: clang/lib/Sema/SemaOpenMP.cpp:201
+      const FieldDecl *FD = nullptr;
+      size_t Sz = -1;
+      VarDecl *VD = nullptr;
----------------
What is Sz here? Better to give a better name and add a description for the 
struct and all fields


================
Comment at: clang/lib/Sema/SemaOpenMP.cpp:1143
+        break;
+      Sz--;
+    }
----------------
Is it possible to have an overflow here?


================
Comment at: clang/lib/Sema/SemaOpenMP.cpp:1148
+    for (const auto &IFD : I->ImplicitDefaultFirstprivateFDs)
+      if (IFD.FD == FD && IFD.Sz == Sz)
+        return IFD.VD;
----------------
What if Sz == -1?


================
Comment at: clang/lib/Sema/SemaOpenMP.cpp:1176
+      }
+      Sz--;
+    }
----------------
What about overflow here?


================
Comment at: clang/lib/Sema/SemaOpenMP.cpp:17480
     DeclRefExpr *Ref = nullptr;
-    if (!VD && !CurContext->isDependentContext())
-      Ref = buildCapture(*this, D, SimpleRefExpr, /*WithInit=*/false);
-    DSAStack->addDSA(D, RefExpr->IgnoreParens(), OMPC_private, Ref);
+    if (!VD && !CurContext->isDependentContext()) {
+      auto *FD = dyn_cast<FieldDecl>(D);
----------------
A check here not for curcontext dependent but for FD being dependent?


================
Comment at: clang/lib/Sema/SemaOpenMP.cpp:17759
     DeclRefExpr *Ref = nullptr;
     if (!VD && !CurContext->isDependentContext()) {
       if (TopDVar.CKind == OMPC_lastprivate) {
----------------
Adjust this check for dependent context with non-dependent FD?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127803

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits