Don't assume _HAS_CONDITIONAL_EXPLICIT for __INTEL_COMPILER #424
Conversation
This change is not a statement of support for the Intel C++ compiler by the STL, so much as an attempt to not break it gratuitously. Fixes DevCom-744112.
| #elif defined(__clang__) | ||
| #define _HAS_CONDITIONAL_EXPLICIT 0 // TRANSITION, LLVM-42694 | ||
| #else // vvv C1XX or non-CUDA EDG vvv | ||
| #elif defined(__clang__) || defined(__CUDACC__) || defined(__INTEL_COMPILER) |
There was a problem hiding this comment.
Note that this is untested, I got __INTEL_COMPILER from the documentation of Intel's C++ compiler. If someone out there has a Windows install of ICC handy and could throw this at it I'd be obliged.
There was a problem hiding this comment.
I think it's fine given our support level for ICC.
|
I'm super uncomfortable trying to support a compiler that we neither support nor test. |
I think you mean "trying to not gratuitously break a compiler that we neither support nor test" ;) |
There was a problem hiding this comment.
I believe that this change is reasonable. While this doesn't change our policy (we neither test nor support ICC), this has near-zero cost and significant benefits for the customers who reported it. The cost is near-zero because we're going to need the fallback codepath for a while (we expect CUDA to follow C1XX's new behavior of unconditionally supporting explicit(bool), but it'll take longer for us to increase our minimum required CUDA version).
However, when we no longer need the fallback for CUDA, I'll want the fallback code to be removed outright - so ICC should implement this cross-vendor behavior in order to continue imitating MSVC.
No, I mean support. This is support. We would be incorrect to resolve the DevCom issue without at least smoke testing that icc works, since it's been broken for ~4 releases now it's likely there are other compiler bugs lurking.
I agree that it is reasonable; I have no objections to the change per se, but I would have objections to resolving the DevCom issue without verifying that ICC does in fact work. |
FWIW, I did try to make this clear in my response on DevCom: We'll make this small change to avoid assuming that ICC has conditional
Whether or not ICC does in fact work is not our problem: we have never claimed it works, and that position isn't changing. |
Description
This change is not a statement of support for the Intel C++ compiler by the STL, so much as an attempt to not break it gratuitously.
Fixes DevCom-744112.
[This is a dual of the internal MSVC-PR-223228.]
Checklist
Be sure you've read README.md and understand the scope of this repo.
If you're unsure about a box, leave it unchecked. A maintainer will help you.
_Uglyas perhttps://eel.is/c++draft/lex.name#3.1 or there are no product code changes.
verified by an STL maintainer before automated testing is enabled on GitHub,
leave this unchecked for initial submission).
members, adding virtual functions, changing whether a type is an aggregate
or trivially copyable, etc.).
the C++ Working Draft (including any cited standards), other WG21 papers
(excluding reference implementations outside of proposed standard wording),
and LWG issues as reference material. If they were derived from a project
that's already listed in NOTICE.txt, that's fine, but please mention it.
If they were derived from any other project (including Boost and libc++,
which are not yet listed in NOTICE.txt), you must mention it here,
so we can determine whether the license is compatible and what else needs
to be done.