kiranchandramohan added a comment.
Had a quick look today. Will spend some more time tomorrow.
Please see a few comments inline.
================
Comment at: clang/include/clang/AST/OpenMPClause.h:6429
+/// and an ordered collection of properties.
+struct OpenMPTraitInfo {
+ struct OpenMPTraitProperty {
----------------
Not clear from this file what should be named starting with OMP or OpenMP. I
see more classes/structs strating with OMP.
================
Comment at: clang/include/clang/Serialization/ASTRecordReader.h:272
+
+ template <> OpenMPTraitInfo *readUserType() { return readOpenMPTraitInfo(); }
+
----------------
Compiler throws up this error.
error: explicit specialization in non-namespace scope ‘class
clang::ASTRecordReader’
================
Comment at: clang/include/clang/Serialization/ASTRecordWriter.h:277
+
+ template <> void writeUserType(OpenMPTraitInfo *TI) {
+ writeOpenMPTraitInfo(TI);
----------------
Compiler throws up this error.
error: explicit specialization in non-namespace scope ‘class
clang::ASTRecordWriter’
================
Comment at: clang/lib/Parse/ParseOpenMP.cpp:853
+ llvm::omp::TraitSelector Selector, llvm::StringMap<SourceLocation> &Seen) {
+ const unsigned Lvl = 2;
+ TIProperty.Kind = TraitProperty::invalid;
----------------
Would it be better to add the value of this variable also to the name? Like
Lvl_TWO or something? Same question for the 5 other const Lvl variables in this
patch.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D71830/new/
https://reviews.llvm.org/D71830
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
