lildmh marked an inline comment as done. lildmh added inline comments.
================ Comment at: lib/Sema/SemaOpenMP.cpp:14805 + if (!Type->isStructureOrClassType() && !Type->isUnionType() && + (MapperIdScopeSpec.isSet() || MapperId.getAsString() != "default")) { + SemaRef.Diag(Loc, diag::err_omp_mapper_wrong_type); ---------------- ABataev wrote: > lildmh wrote: > > ABataev wrote: > > > lildmh wrote: > > > > ABataev wrote: > > > > > lildmh wrote: > > > > > > ABataev wrote: > > > > > > > Why need an additional check for scope and not "default" id? I > > > > > > > don't see this additional requirement in the standard. > > > > > > It's because every variable in map clauses will check this, > > > > > > including those are not struct, class, or union. > > > > > > > > > > > > Using this, e.g., mapping an integer won't report error that it > > > > > > doesn't have a mapper. > > > > > Maybe just move the check to the end of the function? > > > > It will do some additional useless work if I move it to the end. I > > > > don't think it is necessary to move it back. > > > > > > > > But if you believe it's better, I can do it. > > > Could you add tests for this new functionality? > > I already add the test in declare_mapper_message.c. I think nothing is > > needed for ast print test. > I mean, for this error message. I did not see the change in the tests when > you added this new check. line 55, 68, 79 in declare_mapper_messages.c is added for this new check. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67978/new/ https://reviews.llvm.org/D67978 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits