[PATCH] D98558: [OPENMP51]Initial support for the interop directive

[Alexey Bataev via Phabricator via llvm-commits]

ABataev added inline comments.


================
Comment at: clang/include/clang/AST/OpenMPClause.h:7491
+/// \endcode
+class OMPUseClause final : public OMPClause {
+  friend class OMPClauseReader;
----------------
I suggest splitting this patch anв keep just one clause. Other clauses should be added in follow-up patches.


================
Comment at: clang/include/clang/AST/OpenMPClause.h:7620-7623
+  child_range children() { return child_range(&InteropVar, &InteropVar + 1); }
 
   const_child_range children() const {
+    return const_child_range(&InteropVar, &InteropVar + 1);
----------------
This is not always correct. Need to check if this is the extended clause and return this child range only for the extended clause. For the ordinary one need to return an empty range.


================
Comment at: clang/lib/AST/OpenMPClause.cpp:1526
+                              LParenLoc, VarLoc, EndLoc, Exprs.size());
+  std::copy(Exprs.begin(), Exprs.end(), Clause->getTrailingObjects<Expr *>());
+  return Clause;
----------------
`llvm::copy`


================
Comment at: clang/lib/Parse/ParseOpenMP.cpp:2936-2938
+    if (DKind == OMPD_interop)
+      Clause = ParseOpenMPInteropClause(CKind, WrongDirective);
+    else {
----------------
Enclose in braces to keep uniform formatting


================
Comment at: clang/lib/Sema/SemaOpenMP.cpp:14632-14633
+      IsTargetSync = InitClause->getIsTargetSync();
+    else if (const auto *DC = dyn_cast<OMPDependClause>(C))
+      DependClause = DC;
+  }
----------------
```
else {
assert(isa<OMPDependClause>(C) && "...");
...
}
```


================
Comment at: clang/lib/Sema/SemaOpenMP.cpp:14708-14709
+      InteropType = QualType(TD->getTypeForDecl(), 0);
+    } else
+      HasError = true;
+  } else
----------------
Braces


================
Comment at: clang/lib/Sema/SemaOpenMP.cpp:14711
+  } else
+    HasError = true;
+
----------------
Braces


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

https://reviews.llvm.org/D98558