[PATCH] D77812: [flang] Semantic checks for OpenMP combined constructs.

[Peter Klausler via Phabricator via llvm-commits]

klausler requested changes to this revision.
klausler added inline comments.
This revision now requires changes to proceed.


================
Comment at: flang/include/flang/Common/enum-set.h:115
+  constexpr EnumSet operator+(enumerationType v) const {
+    EnumSet result = *this | EnumSet{v};
+    return result;
----------------
Please be consistent with the braced initialization used throughout f18.  It's safer for many types than old-style initialization.


================
Comment at: flang/include/flang/Common/enum-set.h:119
+  constexpr EnumSet operator-(enumerationType v) const {
+    EnumSet result = *this ^ EnumSet { v };
+    return result;
----------------
That's going to add `v` to the set if it's not already present, which seems unintended.


================
Comment at: flang/lib/Semantics/check-omp-structure.cpp:16
 
+static constexpr inline OmpClauseSet doAllowedClauses{OmpClause::PRIVATE,
+    OmpClause::FIRSTPRIVATE, OmpClause::LASTPRIVATE, OmpClause::LINEAR,
----------------
What do you think `inline` is going to do for you when `static` also appears?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77812