[SYCL] Fix set_final_data(..) for vector<bool> iterator

[maximdimakov]

set_final_data() method had compilation error due to lack of the data() method for the std::vector. The fix is needed for support vector
Test: intel/llvm-test-suite#122

Signed-off-by: mdimakov [email protected]

[s-kanaev]

The change looks good in general.
@maximdimakov , could you, please, update description of this patch with link to PR with test at intel/llvm-test-suite?

[vladimirlaz]

Please add test for this case

[s-kanaev]

Looks good overall.

[s-kanaev]

@alexbatashev, @sergey-semenov, could you, please, help reviewing this change?

[alexbatashev]

I'm afraid this is going to break some use cases: https://godbolt.org/z/nznfq1

[maximdimakov]

There is the test SYCL/buffer/buffer.cpp . There are a lot of checks, including pointer (for ex int*, void*). The only compilation error was the error with void*, because there was an iterator_value_type_t and void type was trying to convert to void&. With typename T::value_type all the checks were passed

[alexbatashev]

Ok, sounds good to me

[romanovvlad]

@maximdimakov Could you please provide more details on the root cause of the problem to include it into the final commit message?

[maximdimakov]

@romanovvlad I updated the description. Does it look good?

[romanovvlad]

ated the description. Does it look good?

Yes, thanks.

[keryell]

It is not clear what you are trying to fix here and how you are fixing it since this is uncommented code...
I first thought it was a joke by reading std::vector<bool> in the subject. :-)

[maximdimakov]

@keryell Is it necessary to add comments? This is just the check of vector iterator and then calling the new set_final_data() for it.
Changed the title for more clarity

[keryell]

@keryell Is it necessary to add comments? This is just the check of vector iterator and then calling the new set_final_data() for it.
Changed the title for more clarity

Almost anything with vector of bool is a joke in C++.
So when I see such a code WITHOUT ANY COMMENT, I am just afraid... :-(
What is the problem? Why is it not working? This should be clear in the code...
But it might be also a way you have found to talk about this project on Twitter in the C++ Twitter-sphere, just because it uses a vector of bool... ;-)
@erichkeane can you investigate into this?

[erichkeane]

Not sure what you want me to take a look at, but vector is not a "Container" as it doesn't meet the requirements. It appears to me that this attempts to disallow any std-container of bool (note, it isn't disallowing JUST vector, not sure when it is used otherwise).

It does add the constraint on the type-trait that it can only be used on those std-containers however, since of course it attempts to use the ::value_type of "T" (thus making the is_pointer redundant). I'm guessing this shouldn't have been merged based on that alone.