Conversation
…am since __has_include issue is now WARed
jitify.hpp
Outdated
| << filename << ":" << linenum << std::endl; | ||
| #endif | ||
| // Try loading from filesystem | ||
| bool found_file = false; |
There was a problem hiding this comment.
Not important, but I wonder if this would be cleaner as an immediately-called lambda (bool found_file = [&] { ... }(); so that found_file = true can be replaced with return true etc.).
|
I think I've addressed all the comments, barring the final lambda question. Not sure this is necessarily cleaner. |
jitify.hpp
Outdated
| if (has_name.find("\"", header_start) == std::string::npos) | ||
| throw std::runtime_error("Malformed __has_include statement (" + | ||
| filename + ":" + std::to_string(linenum) + | ||
| ")"); |
There was a problem hiding this comment.
This could be unified between both <> and "" paths by setting a header_end instead of header_count and checking if header_end == std::string::npos after the if/elseif. (It should also probably throw if neither < nor " is found?).
jitify.hpp
Outdated
| if (found_file) { | ||
| line = cleanline.substr(0, has_include_start) + "(1)" + | ||
| cleanline.substr(close + 1); | ||
| } else { | ||
| line = cleanline.substr(0, has_include_start) + "(0)" + | ||
| cleanline.substr(close + 1); | ||
| } |
There was a problem hiding this comment.
A ternary would avoid the code duplication here:
line = cleanline.substr(0, has_include_start) + (found_file ? "(1)" : "(0)") + cleanline.substr(close + 1);
|
Was trying this PR and saw this The line number aside (which might be shifted for some reason), I see libcudacxx has this macro |
This WAR addresses an issue where nvrtc can fail to correctly deal with statements of the form
#if __has_include(<HEADER_NAME>)where even if we have the header present in the set of includes it fails to return true. The WAR here is to search for such statements when we load up headers, and if the header in question in manually found, we replace the statement with
and if not, replace it with
With this WAR in place, we no longer need the unneeded includes in the
builtin_numeric_cuda_std_limits_programprogram code, since these will be automatically included now as per @robertmaynard's original intention.