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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
