[PATCH] D57112: [ASTTypeTraits] OMPClause handling

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri marked 5 inline comments as done.
lebedev.ri added inline comments.


================
Comment at: lib/AST/ASTTypeTraits.cpp:114
+#define OPENMP_CLAUSE(Name, Class)                                             \
+    case OMPC_##Name: return ASTNodeKind(NKI_##Class);
+#include "clang/Basic/OpenMPKinds.def"
----------------
lebedev.ri wrote:
> ABataev wrote:
> > ABataev wrote:
> > > lebedev.ri wrote:
> > > > ABataev wrote:
> > > > > Well, I think it would be good to filter out `OMPC_flush` somehow because there is no such clause actually, it is a pseudo clause for better handling of the `flush` directive.
> > > > > 
> > > > Are `OMPC_threadprivate` and `OMPC_uniform` also in the same boat?
> > > > I don't see those clauses in spec.
> > > > 
> > > > Perhaps `OMPC_flush` should be made more like those other two?
> > > > (i.e. handled outside of `OPENMP_CLAUSE` macro)
> > > `OMPC_threadprivate` is also a special kind of pseudo-clause.
> > > `OMPC_flush` is in the enum, because it has the corresponding class. You can try to exclude it from the enum, but it may require some additional work.
> > > `OMPC_uniform` is a normal clause, but it has the corresponding class. This clause can be used on `declare simd` directive, which is represented as an attribute.
> > I mean, `OOMPC_uniform` has no(!) corresponding class. Mistyped
> As one would expect, simply adding 
> ```
>   case OMPC_flush: // Pseudo clause, does not exist (keep before including .def)
>     llvm_unreachable("unexpected OpenMP clause kind");
> ```
> results in a
> ```
> [58/1118 5.6/sec] Building CXX object tools/clang/lib/AST/CMakeFiles/clangAST.dir/ASTTypeTraits.cpp.o
> FAILED: tools/clang/lib/AST/CMakeFiles/clangAST.dir/ASTTypeTraits.cpp.o 
> /usr/bin/g++  -DGTEST_HAS_RTTI=0 -D_DEBUG -D_GNU_SOURCE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -Itools/clang/lib/AST -I/build/clang/lib/AST -I/build/clang/include -Itools/clang/include -I/usr/include/libxml2 -Iinclude -I/build/llvm/include -pipe -O2 -g0 -UNDEBUG -fPIC -fvisibility-inlines-hidden -Werror=date-time -std=c++11 -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wno-missing-field-initializers -pedantic -Wno-long-long -Wimplicit-fallthrough -Wno-maybe-uninitialized -Wno-class-memaccess -Wno-noexcept-type -Wdelete-non-virtual-dtor -Wno-comment -fdiagnostics-color -ffunction-sections -fdata-sections -fno-common -Woverloaded-virtual -fno-strict-aliasing -pipe -O2 -g0 -UNDEBUG -fPIC   -UNDEBUG  -fno-exceptions -fno-rtti -MD -MT tools/clang/lib/AST/CMakeFiles/clangAST.dir/ASTTypeTraits.cpp.o -MF tools/clang/lib/AST/CMakeFiles/clangAST.dir/ASTTypeTraits.cpp.o.d -o tools/clang/lib/AST/CMakeFiles/clangAST.dir/ASTTypeTraits.cpp.o -c /build/clang/lib/AST/ASTTypeTraits.cpp
> /build/clang/include/clang/Basic/OpenMPKinds.def: In static member function ‘static clang::ast_type_traits::ASTNodeKind clang::ast_type_traits::ASTNodeKind::getFromNode(const clang::OMPClause&)’:
> /build/clang/lib/AST/ASTTypeTraits.cpp:116:5: error: duplicate case value
>      case OMPC_##Name: return ASTNodeKind(NKI_##Class);
>      ^~~~
> /build/clang/include/clang/Basic/OpenMPKinds.def:261:1: note: in expansion of macro ‘OPENMP_CLAUSE’
>  OPENMP_CLAUSE(flush, OMPFlushClause)
>  ^~~~~~~~~~~~~
> /build/clang/lib/AST/ASTTypeTraits.cpp:113:3: note: previously used here
>    case OMPC_flush: // Pseudo clause, does not exist (keep before including .def)
>    ^~~~
> ```
> So one will need to pull `OMPC_flush` out of `clang/Basic/OpenMPKinds.def`.
D57280, will rebase this.


Repository:
  rC Clang

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

https://reviews.llvm.org/D57112