[PATCH] D116764: [clang][OpenMP5.1] Initial parsing/sema for 'indirect' clause

[Alexey Bataev via Phabricator via llvm-commits]

ABataev added inline comments.


================
Comment at: clang/include/clang/Sema/Sema.h:10338
+    Expr *IndirectExpr = nullptr;
+    bool IsIndirect = false;
+
----------------
Can you use `Optional<Expr *>` instead of `Expr *` and `bool`?


================
Comment at: clang/lib/Parse/ParseOpenMP.cpp:1862
+      if (IsIndirectClause) {
+        if (!ParseOpenMPIndirectClause(DTCI, /*ParseOlyn*/false))
+          break;
----------------
`/*ParseOnly=*/`


================
Comment at: clang/lib/Parse/ParseOpenMP.cpp:3427
+
+  DTCI.IndirectExpr = Val.get();
+  if (Val.isInvalid())
----------------
I assume it shall be set only if `Actions.ActOnOpenMPIndirectClause` is successful.


================
Comment at: clang/lib/Sema/SemaOpenMP.cpp:15732
 
+bool Sema::ActOnOpenMPIndirectClause(Expr *Condition,
+                                     SourceLocation StartLoc,
----------------
I assume it shall return `ExprResult`


================
Comment at: clang/lib/Sema/SemaOpenMP.cpp:15747-15748
+      return false;
+    if (Result.getBoolValue() == true)
+      return true;
+  }
----------------
Is this correct? Does `Sema::ActOnOpenMPIndirectClause` is supposed to be failed if `Result` is `false`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116764