Fix calling default constructor to use uniform initialization.#334
Conversation
Signed-off-by: すけや <64895419+sukeya@users.noreply.github.com>
|
This looks fine, thanks. For completeness, could you add a test somewhere ( |
Signed-off-by: Yuya Asano <64895419+sukeya@users.noreply.github.com>
|
I added a test case which fails with the current |
Signed-off-by: Yuya Asano <64895419+sukeya@users.noreply.github.com>
| TEST (testFrustumTest); | ||
| TEST (testInterop); | ||
| TEST (testNoInterop); | ||
|
|
There was a problem hiding this comment.
This needs a #ifdef __CUDACC__. That appears to be the reason the CI build fails.
There was a problem hiding this comment.
I think __CUDACC__ is defined when nvcc compiles device code, so it never be defined in host code.
There was a problem hiding this comment.
That's the point, when compiling the code without CUDA, it's currently failing because testVecCUDA isn't defined. Check the errors in the log from the failed CI job: https://github.com/AcademySoftwareFoundation/Imath/actions/runs/5599599937/jobs/10240779807?pr=334#step:6:105
Thanks!
There was a problem hiding this comment.
I fixed. I'm sorry, but could you please review?
Signed-off-by: Yuya Asano <64895419+sukeya@users.noreply.github.com>
Signed-off-by: Yuya Asano <64895419+sukeya@users.noreply.github.com>
Signed-off-by: Yuya Asano <64895419+sukeya@users.noreply.github.com>
|
We discussed this in today's technical steering committee meeting, and we support this contribution in principle and we'd like to work towards accepting it. We'd like to confirm what versions of CUDA this relies on, and confirm that the #ifdef's properly exclude ones it doesn't. We'd also like to add a CUDA validation job to the CI. We are planning a v3.2 release in mid-August, which will be cut from the main branch, so we'd like to wait until after that before merging the PR. That will provide more time for evaluation before official release. In spite of my earlier advice to add a test, your original submission to switch to uniform initialization is fairly uncontroversial, so if you'd like to submit that as a separate PR, we can accept that right away, while we work through the issues with the addition of the CUDA test. Again, thanks for your contribution and your patience. |
2 participants
Hello,
I faced an error that let
Cbe a template parameter,C()is interpreted as a function call whenCis a type alias.So, I replaced all ambiguous expression
C()to uniform initializationC{}.Could you please merge this PR?