[PATCH] D74970: [OpenMP] Refactor the analysis in checkMapClauseBaseExpression using StmtVisitor class.

[Chi Chun Chen via Phabricator via cfe-commits]

cchen added a comment.

In D74970#1887252 <https://reviews.llvm.org/D74970#1887252>, @ABataev wrote:

> In D74970#1887246 <https://reviews.llvm.org/D74970#1887246>, @cchen wrote:
>
> > In D74970#1887193 <https://reviews.llvm.org/D74970#1887193>, @ABataev wrote:
> >
> > > Did you try to run the tests? I would suggest adding a test, at least, where a function is mapped. We should see the error message here. Seems to me, we don't have this one.
> >
> >
> > We already have test for `err_omp_invalid_map_this_expr`, `note_omp_invalid_subscript_on_this_ptr_map`, etc.. in target_message.cpp line 44 without checking for value dependence. Do you mean that I should add a test for the check of value dependence? In that case, we don't print any messages.
>
>
> No. But previously, if we tried to map a function, not a variable, it would be skipped silently. With this patch, this behavior will change. I suggest adding a test with mapping a function to check that error message is emitted. Something like:
>
>   int foo();
>   #pragma omp target map(foo) // <- must be an error here
>
>
> And I just asked if you tried to run the tests with this patch. Did the result change or it is the same?


Got it, I'll add test for function mapping. I've run the test and this patch passed all the test.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74970