[PATCH] D74941: [OpenMP] `omp begin/end declare variant` - part 1, parsing

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert marked 6 inline comments as done.
jdoerfert added a comment.

In D74941#1887367 <https://reviews.llvm.org/D74941#1887367>, @fpetrogalli wrote:

> 1. I am not familiar with `OpenMP technical report 8 (TR8)`. Could you please provide a link in the description? Same for PR42061, PR42798, PR42799. It would be useful to get an idea of the syntax.


Added a link in the commit message. Also to some other commit that might be interesting to see that this is a long time coming.
Wrt. the syntax, arguably with this patch clang actually tells you the syntax: https://godbolt.org/z/Cjomow

> 2. It is my understanding that this code cover for functionalities that are still under development in the OpenMP standard. To avoid confusion when reading the code base, I think it would be worth marking the enumerations, the error message and (when possible) the methods with an extra descriptor that marks them as experimental. For example, rename `OMPD_begin_declare_variant` to `OMPD_experimental_begin_declare_variant`, or something similar. In the (admittedly unlikely) event the feature doesn't get in future releases of the standard, it will be easier to clean up the code.

I have strong reservations against doing that (here). Let me give you the top two:

1. This will go into 5.1.
2. Things we name "experimental" are traditionally *never* renamed.



> 3. This is parsing - why did you have to add also code in the semantic part of the compiler?

Because I wanted to avoid warnings due to non-exhaustive switch statements :)



================
Comment at: clang/include/clang/Basic/DiagnosticParseKinds.td:1251
   "expected '#pragma omp end declare target'">;
+def err_expected_end_declare_target_or_variant : Error<
+  "expected '#pragma omp end declare %select{target|variant}0'">;
----------------
kkwli0 wrote:
> Can we merge it with err_expected_end_declare_target?
Did that and forgot to delete it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74941