ABataev added inline comments.
================ Comment at: clang/lib/Parse/ParseOpenMP.cpp:3067-3068 + unsigned Type = getOpenMPSimpleClauseType(OMPC_map, PP.getSpelling(Tok)); + if (Type < OMPC_MAP_MODIFIER_unknown) + return OMPC_MAP_MODIFIER_unknown; + return static_cast<OpenMPMapModifierKind>(Type); ---------------- jdenny wrote: > ABataev wrote: > > Why do we need this? > When called with `OMPC_map`, `getOpenMPSimpleClauseType` can return either an > `OpenMPMapModifierKind` or `OpenMPMapClauseKind` depending on `Tok`. Thus, > without this change, both `isMapModifier` and `isMapType` can return either > of those despite the difference in their names and documentation. > > I found that, when `Parser::parseMapTypeModifiers` ignores `present` as if > it's not a modifier because OpenMP < 5.1, then `isMapType` later returns > `OMPC_MAP_MODIFIER_present` and causes the following assert to fail in > `Sema::ActOnOpenMPVarListClause`: > > ``` > assert(0 <= ExtraModifier && ExtraModifier <= OMPC_MAP_unknown && > "Unexpected map modifier."); > ``` > > To me, the most obvious solution is to fix `isMapType` and `isMapModifier`. > Fortunately, looking in `OpenMPKinds.h`, the enumerator values in > `OpenMPMapModifierKind` and `OpenMPMapClauseKind` are disjoint so we can tell > which we have. Can we have something like: ``` if (LangOpts.OpenMP <= 50 && Type == OMPC_MAP_MODIFIER_present) return OMPC_MAP_MODIFIER_unknown; ``` or extend `getOpenMPSimpleClauseType` function with the version parameter and check there is modifier is allowed and return `unknown` if it is not compatible with provided version? ================ Comment at: clang/lib/Parse/ParseOpenMP.cpp:3103 /// map([ [map-type-modifier[,] [map-type-modifier[,] ...] map-type : ] list) /// where, map-type-modifier ::= always | close | mapper(mapper-identifier) bool Parser::parseMapTypeModifiers(OpenMPVarListDataTy &Data) { ---------------- Update a comment here ================ Comment at: clang/test/OpenMP/target_data_codegen.cpp:258-260 +// CK1A: [[MTYPE00:@.+]] = {{.+}}constant [1 x i64] [i64 [[#0x1021]]] + +// CK1A: [[MTYPE01:@.+]] = {{.+}}constant [1 x i64] [i64 [[#0x1425]]] ---------------- Add a comment with interpretaion of the map flags, like `OMP_TO = 0x1 | OMP_FROM=0x2 | OMP_PRESENT = 0x1000 = 0x1003` CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83061/new/ https://reviews.llvm.org/D83061 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits