rjmccall added a comment.

In D65286#1606441 <https://reviews.llvm.org/D65286#1606441>, @Anastasia wrote:

> In D65286#1606071 <https://reviews.llvm.org/D65286#1606071>, @mantognini 
> wrote:
>
> > In `vector_literals_nested.cl`, we have tests for (global) constants. Do 
> > you think it would be worth testing those as well in C++ mode? Maybe the 
> > two files (`vector_literals_nested.cl` and `vector_literals_valid.cl`) 
> > should also be merged as most of their content seems duplicated.
>
>
> Thanks for pointing out. Indeed there was lots of repetition. I only added 
> one missing test case (line 37 of `vector_literals_valid.cl`) and also 
> removed another similar file `test/SemaOpenCL/vector_literals_const.cl`.
>
> > In C++, we have the comma operator and therefore the AST is significantly 
> > different. Running the content of your test file with `-x c++` shows that 
> > it is rejected as desired. It could be worth having a negative test to 
> > ensure we always reject this in vanilla C++.
>
> Added test case in `test/SemaCXX/vector.cpp`. However I am now confused what 
> syntax we shouldn't accept exactly. @rjmccall  do you think there should be 
> an error on line  341?


No, it's correct that this is accepted as a vector splat.  The warning is good, 
although it would be even better if we could find a way to warn about this more 
specifically, like "warning: this syntax does not construct a vector; initial 
elements are ignored" (needs workshopping, obviously).


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

https://reviews.llvm.org/D65286



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to