[PATCH] D123235: [OpenMP] atomic compare fail : Parser & AST support

[Shilei Tian via Phabricator via llvm-commits]

tianshilei1992 added a comment.

LGTM, but please fix the build error first.



================
Comment at: clang/include/clang/AST/ASTNodeTraverser.h:228
 
+  void Visit(const OMPFailClause *C) {
+    getNodeDelegate().AddChild([=] {
----------------
koops wrote:
> tianshilei1992 wrote:
> > koops wrote:
> > > tianshilei1992 wrote:
> > > > Why would we want a dedicated function since it is only called once?
> > > The code for this method cannot be put into any other method because it handles only OMPFailClause. All other Visit methods handle either the generalized OMPClause or other types of Clauses.
> > I mean, it's only used by the function above, no?
> I agree with you but, I cannot understand any better way of coding it.
Initially I thought to merge them, but this looks fine.


================
Comment at: clang/include/clang/AST/OpenMPClause.h:2305
+    OpenMPClauseKind *MOCK = getTrailingObjects<OpenMPClauseKind>();
+    //*getTrailingObjects<OpenMPClauseKind>() = memOrder;
+    *MOCK = MemOrder;
----------------
If the code is commented out, plz remove it.


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

https://reviews.llvm.org/D123235