jdenny marked 4 inline comments as done. jdenny 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); ---------------- 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. ================ Comment at: clang/lib/Parse/ParseOpenMP.cpp:3150-3151 + unsigned Type = getOpenMPSimpleClauseType(OMPC_map, PP.getSpelling(Tok)); + if (Type > OMPC_MAP_unknown) + return OMPC_MAP_unknown; + return static_cast<OpenMPMapClauseKind>(Type); ---------------- ABataev wrote: > Same, why do we need this? Responded on `isMapModifier`. 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