Anastasia added a comment.

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?

> Regarding the code itself, I'm not familiar with 
> `InitializationSequence`/`InitListChecker` that much, but the patch makes 
> sense I would say.

Ok, thanks. It seems apparently the last time this code was modified in 2015... 
so might not be easy to remember details for anyone. :(


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