[PATCH] D111854: [OPENMP51]Initial parsing/sema for append_args clause for 'declare variant'

[Mike Rice via Phabricator via llvm-commits]

mikerice added inline comments.


================
Comment at: clang/lib/Parse/ParseOpenMP.cpp:1496-1497
+  bool HasError = false;
+  bool IsTarget = false;
+  bool IsTargetSync = false;
+
----------------
ABataev wrote:
> Can you worjk with OMPDeclareVariantAttr::InteropType type var instead of using these vars?
Could do that, but it seems to make the code uglier.  Instead of 


```
if (IsTarget)
  Diag
isTarget = true;
```

We'd get:

```
if (IT == OMPDeclareVariantAttr::Target ||          
    IT == OMPDeclareVariantAttr::Target_TargetSync)
  Diag
if (IT == OMPDeclareVariantAttr::TargetSync)
  IT = OMPDeclareVariantAttr::Target_TargetSync;
else    
  IT = OMPDeclareVariantAttr::Target;
``` 

And again for the targetsync case.

Plus there is no unset value in OMPDeclareVariantAttr::Interop type so we'd need to add one (complicating other code) or initialize it with (OMPDeclareVariantAttr::InteropType)-1 or something.


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

https://reviews.llvm.org/D111854