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

Reply via email to