[PATCH] D91980: [OpenMP] Add initial support for `omp [begin/end] assumes`

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.


================
Comment at: clang/include/clang/Basic/DiagnosticParseKinds.td:1296-1297
+    : Error<"'#pragma omp end assumes' with no matching '#pragma omp begin assumes'">;
+def err_expected_assumption_clause
+    : Error<"'%0' directive requires at least one clause">;
+def warn_omp_unknown_assumption_clause_missing_id
----------------
There is `err_omp_no_clause_for_directive` defined in Sema. See `Sema::ActOnOpenMPTargetDataDirective` how to use it, better to reduce the number of diagnostics if possible


================
Comment at: clang/lib/Sema/SemaLambda.cpp:999-1002
+  // OpenMP lambdas might get assumumption attributes.
+  if (LangOpts.OpenMP)
+    ActOnFinishedFunctionDefinitionInOpenMPAssumeScope(Method);
+
----------------
Are there any other function-like constructs that also should have markings? Coroutines maybe? 


================
Comment at: clang/lib/Sema/SemaOpenMP.cpp:3203
+  auto *AA = AssumptionAttr::Create(Context, llvm::join(Assumptions, ","), Loc);
+  // Disable assumes in OpenMP simd mode.
+  if (DKind == llvm::omp::Directive::OMPD_begin_assumes) {
----------------
How this comment relates to the code?


================
Comment at: clang/lib/Sema/SemaOpenMP.cpp:3207
+  } else {
+    assert(DKind == llvm::omp::Directive::OMPD_assumes && "");
+    OMPAssumeGlobal.push_back(AA);
----------------
Add a message in for the assertion


================
Comment at: clang/lib/Sema/SemaOpenMP.cpp:3217
+    while (Ctx->getParent())
+      Ctx = Ctx->getParent();
+    DeclContexts.push_back(Ctx);
----------------
Maybe, better to use `getLexicalParent`? `getParent` returns semantic parent, while `getLexicalParent` - the lexical parent. Do you need to mark the declarations in the lexical context or in the semantic context?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91980