[PATCH] D127803: Generate the capture for field when the field is used with implicit default.

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.


================
Comment at: clang/lib/Sema/SemaExprMember.cpp:1873-1877
+    if (auto *PrivateCopy =
+            isOpenMPFDCaptureDecl(Field, Base.get(), IsArrow, OpLoc, &SS,
+                                  /*TemplateKWLoc=*/SourceLocation(), Field,
+                                  FoundDecl, /*HadMultipleCandidates=*/false,
+                                  MemberNameInfo, MemberType, VK, OK))
----------------
Why do we need this function? Implicit private rule should apply (if should) only to this-Юашдув kind of expressions, if something like parent.field expression is used, parent should be private, not private.field. Or I'm missing something?


================
Comment at: clang/lib/Sema/SemaOpenMP.cpp:201
+    using ImplicitFDCapTy = std::pair<ImpilitFDLevelTy, VarDecl *>;
+    // List of captuer fields
+    llvm::SmallVector<ImplicitFDCapTy, 8> ImplicitDefaultFirstprivateFDs;
----------------
///


================
Comment at: clang/lib/Sema/SemaOpenMP.cpp:1129-1130
   }
+  // get captured field from ImplicitDefaultFirstprivateFDs
+  VarDecl *getImplicitFDCapExprDecl(const FieldDecl *FD) {
+    const_iterator I = begin();
----------------
1. Use /// style of comment.
2. const member function?


================
Comment at: clang/lib/Sema/SemaOpenMP.cpp:1141
+      return nullptr;
+    for (auto IFD : I->ImplicitDefaultFirstprivateFDs)
+      if (IFD.first.first == FD && IFD.first.second == Sz)
----------------
`const auto &`?


================
Comment at: clang/lib/Sema/SemaOpenMP.cpp:1146-1147
+  }
+  // Check if capture decl is field captured in ImplicitDefaultFirstprivateFDs
+  bool isImplicitDefaultFirstprivateFD(VarDecl *VD) {
+    const_iterator I = begin();
----------------
1. Use /// style of comment.
2. const member function?


================
Comment at: clang/lib/Sema/SemaOpenMP.cpp:1155
+      return false;
+    for (auto IFD : I->ImplicitDefaultFirstprivateFDs)
+      if (IFD.second == VD)
----------------
`const auto &`?


================
Comment at: clang/lib/Sema/SemaOpenMP.cpp:1160
+  }
+  // Store capture FD info in ImplicitDefaultFirstprivateFDs
+  void addImplicitDefaultFirstprivateFD(const FieldDecl *FD, VarDecl *VD) {
----------------
/// style


================
Comment at: clang/lib/Sema/SemaOpenMP.cpp:1167-1168
+      if (I->DefaultAttr == DSA_private || I->DefaultAttr == DSA_firstprivate) {
+        I->ImplicitDefaultFirstprivateFDs.emplace_back(std::make_pair(FD, Sz),
+                                                       VD);
+        break;
----------------
just `emplace_back(FD, Sz, VD);`?


================
Comment at: clang/lib/Sema/SemaOpenMP.cpp:2410
         D,
-        [](OpenMPClauseKind C, bool AppliedToPointee) {
+        [](OpenMPClauseKind C, bool AppliedToPointee, ...) {
           return isOpenMPPrivate(C) && !AppliedToPointee;
----------------
Better to add an actual param type here but without Param name


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127803