ABataev added a comment.

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?


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

Reply via email to