[PATCH] D67978: [OpenMP 5.0] Fix user-defined mapper lookup in sema for arrays

[Lingda Li via Phabricator via cfe-commits]

lildmh added a comment.

In D67978#1684169 <https://reviews.llvm.org/D67978#1684169>, @ABataev wrote:

> In D67978#1684104 <https://reviews.llvm.org/D67978#1684104>, @lildmh wrote:
>
> > In D67978#1683166 <https://reviews.llvm.org/D67978#1683166>, @ABataev wrote:
> >
> > > In D67978#1683146 <https://reviews.llvm.org/D67978#1683146>, @lildmh wrote:
> > >
> > > > HI Alexey, the ast print test is already there. Because I didn't check the mapper for array type before, such code will always not report any error, and ast print test is correct. Codegen test belongs to the other patch I released. It fits that patch much better.
> > >
> > >
> > > How is this possible? If we did not have support for the array type, we could not have correct handling of such types in successful tests.
> >
> >
> > The ast print for array with mapper was correct because the mapper id is still with the array type. Without this patch, the problem is it will not look up the mapper declaration associated with the id, and as a result, the codegen is not correct. I found this problem when I tested the codegen.
>
>
> Then another one question. Why we don't emit the diagnostics if the original type is not a class, struct or union? We just ignore this situation currently but we should not. The error message must be emitted. I think that's why the ast-print test works correctly though it should not.


We emit such diagnostic when mapper is declared, i.e., you cannot define a mapper for a non-class, non-struct, non-union type. It's in function `ActOnOpenMPDeclareMapperType` in SemaOpenMP.cpp. But I didn't emit such information where mapper is used. I will fix it in this patch. Thanks for getting this!


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67978/new/

https://reviews.llvm.org/D67978