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

[Mike Rice via Phabricator via llvm-commits]

mikerice marked 4 inline comments as done.
mikerice added inline comments.


================
Comment at: clang/include/clang/AST/OpenMPClause.h:7423-7424
+  /// preference-list.
+  /// \param IsTarget Uses the 'target' interop-type.
+  /// \param IsTargetSync Uses the 'targetsync' interop-type.
+  /// \param StartLoc Starting location of the clause.
----------------
ABataev wrote:
> Do we really need these params? I assume they are already part of `Exprs` though in `Expr *` form.
The 'target' and 'targetsync' interop-types are not parsed as Exprs.  Are you suggesting we should create Exprs for these?


================
Comment at: clang/include/clang/AST/OpenMPClause.h:7491
+/// \endcode
+class OMPUseClause final : public OMPClause {
+  friend class OMPClauseReader;
----------------
ABataev wrote:
> mikerice wrote:
> > ABataev wrote:
> > > I suggest splitting this patch anв keep just one clause. Other clauses should be added in follow-up patches.
> > Sorry I should have done that.  Unfortunately the logic is somewhat shared among these three and breaking it up at this point would mean rewriting it without those interactions.  I can do it if you really want me to.
> Better to split, if possible.
Should be possible.  Is it better to start a new review or just update this one?


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

https://reviews.llvm.org/D98558