nemequ / hedley

A C/C++ header to help move #ifdefs out of your code
https://nemequ.github.io/hedley/
Creative Commons Zero v1.0 Universal
774 stars 51 forks source link

Incompatible detection for Intel Compiler on windows #39

Closed mizvekov closed 4 years ago

mizvekov commented 4 years ago

Hedley correctly detects Intel compiler and version, but then uses it to enable usage of GCC dialects like attribute(foo) which is not supported on windows, where the compiler is going for MSVC compatibility instead of GCC. Likewise it is not making use of the parts of the MSVC dialect which are supported by Intel on that platform.

nemequ commented 4 years ago

Interesting, thanks for the report. I thought ICC just supported both versions everywhere; IIRC you can use several MSVC-style things on Linux. I'll try to get a copy of ICC installed on a Windows VM over the weekend, and hopefully make a new release early next week.

nemequ commented 4 years ago

I installed Intel oneAPI on a Windows VM, and I'm unable to reproduce this. Just like on Linux, it supports both attribute and declspec syntaxes.

Can you please provide a reproducible test case, including the exact command you're using to invoke the compiler? If it helps, there are lots of tests in the test/ subdirectory.

mizvekov commented 4 years ago

Hello, I can try to make a reproducible test case for you. But what I have exactly now is that I discovered this problem when using the nlohmann/json project, which bundles the latest version (13) of hedley. This is the error I had:

C:\PROGRA~2\INTELS~1\SW_DEV~1\COMPIL~1.218\windows\bin\intel64\icl.exe  /nologo /TP /DWIN32 /D_WINDOWS /D_CRT_SECURE_NO_WARNINGS /W3 /Zi /bigobj /Oi /MP /GF /Gy /permissive- /EHsc /WX- /wd1292 /Ox /Ob2 /DNDEBUG /Ot /GS- /fp:fast -MT   -Qstd=c++20 /showIncludes /Fomain.cc.obj -c main.cc
nlohmann\detail\exceptions.hpp(60): error : expected a type specifier
        JSON_HEDLEY_NON_NULL(3)
        ^

nlohmann\detail\exceptions.hpp(60): error #303: explicit type is missing ("int" assumed)
        JSON_HEDLEY_NON_NULL(3)
        ^

nlohmann\detail\exceptions.hpp(61): error : expected a ";"
        exception(int id_, const char* what_arg) : id(id_), m(what_arg) {}

And that investigating this, hedley was providing this definition based on the fact it detected an Intel C++ compiler:

#define JSON_HEDLEY_NON_NULL(...) __attribute__((__nonnull__(__VA_ARGS__)))

Which the compiler was not accepting.

Testing the following snippet:

static void* test(void* in) __attribute__((__nonnull__));

in another part of my project, which does not use hedley, I got the following error:

r3k.cc(947): error : expected a "{"
    static void* test(void* in) __attribute__((__nonnull__));

Patching hedley to short circuit the intel compiler detection was my temporary way around the problem.

Have you tested the compiler yourself through the icl frontend (which mimics MSVC's cl) ?

mizvekov commented 4 years ago

I realized you might be testing a different C++ compiler than I am. I am testing Intel C++ compiler 19.1 for windows (which is included in System Studio): https://software.intel.com/content/www/us/en/develop/tools/system-studio/choose-download.html You can get this with a free renewable one year license .

This intel oneAPI DPC++ is something I didn't know and I just heard about from yourself.

nemequ commented 4 years ago

As soon as I saw the command you're using I saw the problem; I was using icx not icc. AFAICT icx is basically their next-generation clang-based compiler, which looks like it will support both attribute and declspec just like icc does.

You can get icc through oneAPI (it's in the HPC add-ons); DPC++ is something else.

I'm going to need to think about the API a bit. I'm leaning towards HEDLEYINTEL basically being icc-only, and adding a separate HEDLEYICL group for icl. Interestingly, neither icc -qnext-gen nor icx define __INTEL_COMPILER, so icc -qnext-gen will use the clang paths. The biggest question is whether HEDLEY_ICL_* should apply to icx…

nemequ commented 4 years ago

I have added full support for icl, sorry about the delay. It's in the dev branch now, and I'm planning on putting out a release in the next day or two.

nlohmann commented 4 years ago

@nemequ Do you have a rough idea when the next release will be published?

nemequ commented 4 years ago

@nlohmann, I can do it today if you want. I was kind of hoping to get a better idea of how an issue with warning numbers on PGI will be resolved, but the code I have in place right now will just disable both warning numbers which should be stable going forward, and I can always tweak it in the next release to only disable one if PGI fixes it.

Edit: to be clear, all we would lose by releasing now is that on future versions of PGI might disable more diagnostics than are strictly necessary until Hedley is updated again so we test for PGI == 20.7 instead of PGI >= 20.7 when determining which diagnostic numbers to disable. In other words, a relatively small price to pay for releasing now.

nlohmann commented 4 years ago

I did not mean to rush you. It's just that I have nlohmann/json#2346 which depends on this fix, and I would rather not make a release with the develop version of Hedley.

nemequ commented 4 years ago

No worries, I understand. If you want to make a release I can go ahead and put one out first; I've done pretty much all of the necessary testing already, so it's really not a big deal.

Waiting on PGI is probably a bit silly; it's not exactly a popular compiler, and the only disadvantage to putting out a release now is that future versions of the compiler will disable one more diagnostic then they have to (i.e., using HEDLEY_DIAGNOSTIC_DISABLE_UNKNOWN_CPP_ATTRIBUTES will disable diagnostics 1097 and 1098, when in reality we only want to disable one of them, I'm just not sure which one we want to disable until I find out if PGI is going to go back to the old diagnostic numbers or proceed with the new ones.

I'll go ahead and finish up my testing with the current dev branch this afternoon. Then if you can just give me a little warning before you want to make a new release I'll make sure to put out a new version of Hedley (which is trivial once the testing is done).

nemequ commented 4 years ago

@nlohmann, I just release v14, it's exactly the same as what has been in dev for a while.