Fixed compile error and switch case warning #138
Conversation
| @@ -1680,9 +1680,9 @@ class LibNvJitLink | |||
| #if CUDA_VERSION >= 12030 | |||
| case NVJITLINK_ERROR_THREADPOOL: return "NVJITLINK_ERROR_THREADPOOL"; | |||
| #endif | |||
| default: return "(unknown nvJitLink error)"; | |||
There was a problem hiding this comment.
I think this will prevent warnings in future when CUDA adds new enums (e.g., NVJITLINK_ERROR_UNRECOGNIZED_INPUT, which I added in the WIP branch but not here).
Is there another way to avoid the warning you're seeing?
There was a problem hiding this comment.
I think that was the missing enum, I'll check.
Which branch is that and what's the jitify2 merge timeline like?
I don't think there's any other way other than adding the enum or having a default case.
There was a problem hiding this comment.
I'm hoping to merge that PR in the near future, and then merge jitify2 into master after that.
| : Option(output_key, compiler_option.value()); | ||
| output_key.empty() | ||
| ? compiler_option | ||
| : Option(std::string{output_key}, compiler_option.value()); |
There was a problem hiding this comment.
Thanks for this fix. (I didn't see it because our tests are still set to C++11; we should probably bump to 17).
There was a problem hiding this comment.
yeah, it would be helpful to test for all supported C++ versions, otherwise compile errors like that will eventually happen
This merge requests fixes the compile error in CUDA 12.* where the Option's
std::stringconstructor is called with aStringRef(std::string_view) which is not a valid implicit conversion.GCC also is not convinced that all branches in jitify2.hpp:1682 are taken, and thus raises a warning, we have warnings as errors in CUDF so it breaks our build.