ABataev added inline comments.
================
Comment at: include/clang/AST/OpenMPClause.h:2628
@@ +2627,3 @@
+ friend class OMPClauseWriter;
+ friend class Sema;
+
----------------
I don't like the idea of friend Sema. If Sema needs some interfaces, they must
be public.
================
Comment at: include/clang/Basic/OpenMPKinds.h:83
@@ +82,3 @@
+enum OpenMPMapClauseKind {
+ OMPC_MAP_unknown = 0,
+#define OPENMP_MAP_KIND(Name) \
----------------
This must be the last element in enum.
================
Comment at: include/clang/Basic/OpenMPKinds.h:87
@@ +86,3 @@
+#include "clang/Basic/OpenMPKinds.def"
+ NUM_OPENMP_MAP_KIND
+};
----------------
Remove this one, it is not required
================
Comment at: include/clang/Sema/Sema.h:8013-8014
@@ -8013,1 +8012,4 @@
+ OpenMPLinearClauseKind LinKind, OpenMPMapClauseKind MapTypeModifier,
+ OpenMPMapClauseKind MapType, SourceLocation DepLinLoc,
+ SourceLocation MapLoc);
/// \brief Called on well-formed 'private' clause.
----------------
I think DepLinLoc and MapLoc can joined into one argument, like DepLinMapLoc
================
Comment at: lib/AST/StmtPrinter.cpp:853-854
@@ +852,4 @@
+ }
+ OS << getOpenMPSimpleClauseTypeName(OMPC_map, Node->getMapType());
+ OS << ':';
+
----------------
I think MapType can be optional, just like MapTypeModifier
================
Comment at: lib/AST/StmtPrinter.cpp:856-858
@@ +855,5 @@
+
+ for (OMPMapClause::varlist_iterator I = Node->varlist_begin(),
+ E = Node->varlist_end();
+ I != E; ++I) {
+ OS << (I == Node->varlist_begin() ? ' ' : ',')
----------------
Better to use ranged for, if possible
================
Comment at: lib/Parse/ParseOpenMP.cpp:780-781
@@ -772,2 +779,4 @@
+ bool UnexpectedId = false;
SourceLocation DepLinLoc;
+ SourceLocation MapLoc;
----------------
These two can be joined
================
Comment at: lib/Sema/SemaOpenMP.cpp:7482-7484
@@ +7481,5 @@
+ bool IsCorrect = true;
+ for (DeclContext::decl_iterator I = DC->noload_decls_begin(),
+ E = DC->noload_decls_end();
+ I != E; ++I) {
+ if (*I) {
----------------
Use ranged loop and do not use noload version.
================
Comment at: lib/Sema/SemaOpenMP.cpp:7503-7505
@@ +7502,5 @@
+ }
+ for (CXXRecordDecl::base_class_iterator I = RD->bases_begin(),
+ E = RD->bases_end();
+ I != E; ++I) {
+ if (!IsCXXRecordForMappable(SemaRef, I->getLocStart(), Stack,
----------------
Ranged loop
================
Comment at: lib/Sema/SemaOpenMP.cpp:7507-7509
@@ +7506,5 @@
+ if (!IsCXXRecordForMappable(SemaRef, I->getLocStart(), Stack,
+ I->getType()->getAsCXXRecordDecl())) {
+ IsCorrect = false;
+ }
+ }
----------------
Braces are not needed
================
Comment at: lib/Sema/SemaOpenMP.cpp:7522-7524
@@ +7521,5 @@
+ if (!RD->isInvalidDecl() &&
+ !IsCXXRecordForMappable(SemaRef, SL, Stack, RD)) {
+ return false;
+ }
+ }
----------------
Extra braces
================
Comment at: lib/Sema/SemaOpenMP.cpp:7636-7638
@@ +7635,5 @@
+ if (!CheckTypeMappable(VE->getExprLoc(), VE->getSourceRange(), *this,
+ DSAStack, Type)) {
+ continue;
+ }
+
----------------
Extra braces
================
Comment at: test/OpenMP/target_map_messages.cpp:1
@@ +1,2 @@
+// RUN: %clang_cc1 -verify -fopenmp -ferror-limit 100 %s
+
----------------
Tests for templates, serialization/deserialization, AST printing, region nesting
http://reviews.llvm.org/D14134
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
