fpetrogalli added a comment.

Hi @jdoerfert ,

thank you for working on this.

I have a couple of high level comments.

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.

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.

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

Thanks!

Francesco


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74941



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to