Skip to content

alias template error with Intel 2016.0.3 compilers #1121

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
loriab opened this issue Sep 29, 2017 · 28 comments
Closed

alias template error with Intel 2016.0.3 compilers #1121

loriab opened this issue Sep 29, 2017 · 28 comments
Milestone

Comments

@loriab
Copy link
Contributor

loriab commented Sep 29, 2017

Issue description

In trying to update some projects from 2.0.0 to 2.2.1, I've found that they consistently won't compile with the icpc (ICC) 16.0.3 2016041 line of compilers. Fine with GCC 5.2 and Intel icpc (ICC) 17.0.4 20170411. An error is below:

In file included from /home/psilocaluser/gits/cmake_example/pybind11/include/pybind11/attr.h(13),
                 from /home/psilocaluser/gits/cmake_example/pybind11/include/pybind11/pybind11.h(43),
                 from /home/psilocaluser/gits/cmake_example/src/main.cpp(1):
/home/psilocaluser/gits/cmake_example/pybind11/include/pybind11/cast.h(837): error: alias template "pybind11::detail::type_caster_base<type>::cast_op_type" may not be used in the type-id of the alias-declaration
      template <typename T> using cast_op_type = cast_op_type<T>;
                                                 ^

In file included from /home/psilocaluser/gits/cmake_example/pybind11/include/pybind11/attr.h(13),
                 from /home/psilocaluser/gits/cmake_example/pybind11/include/pybind11/pybind11.h(43),
                 from /home/psilocaluser/gits/cmake_example/src/main.cpp(1):
/home/psilocaluser/gits/cmake_example/pybind11/include/pybind11/cast.h(1814): warning #3373: nonstandard use of "auto" to both deduce the type from an initializer and to announce a trailing return type
      static constexpr auto args_pos = constexpr_first<argument_is_args, Args...>() - (int) sizeof...(Args),
                       ^

compilation aborted for /home/psilocaluser/gits/cmake_example/src/main.cpp (code 2)
gmake[2]: *** [CMakeFiles/cmake_example.dir/src/main.cpp.o] Error 2

Reproducible example code

The error can be seen with @dean0x7d's sample cmake+pybind11 repo, https://github.com/pybind/cmake_example (admittedly pinned at 2.2.0). When forced to switch compiler families as below, it gives the error above.

>>> icpc --version
icpc (ICC) 16.0.3 20160415
Copyright (C) 1985-2016 Intel Corporation.  All rights reserved.
>>> mkdir build/temp.linux-x86_64-3.5
>>> cd build/temp.linux-x86_64-3.5 &&  cmake /path/to/cmake_example -DCMAKE_LIBRARY_OUTPUT_DIRECTORY=/path/to/cmake_example/build/lib.linux-x86_64-3.5 -DPYTHON_EXECUTABLE=/miniconda/envs/idp35p4/bin/python -DCMAKE_BUILD_TYPE=Release -DCMAKE_C_COMPILER=icc -DCMAKE_CXX_COMPILER=icpc
>>> cmake --build . --config Release -- -j2

I'm sorry to not further narrow the error source. I'm glad to try out any changes since this particular compiler flavor may not be widespread.

@loriab loriab mentioned this issue Sep 29, 2017
7 tasks
@klaw9
Copy link

klaw9 commented Oct 3, 2017

This issue also affects nvcc (cuda 8). For now I made it work by adding a namespace qualifier to the type-id of the alias statement.

template <typename T> using cast_op_type = detail::cast_op_type<T>;

@loriab
Copy link
Contributor Author

loriab commented Oct 11, 2017

Thanks, @klaw9, I confirm that fixes the stated problem.

Next problem is below, if anyone's curious.

/home/psilocaluser/gits/hrw-matt/objdir_aggh2016/stage/usr/local/psi4/include/pybind11/detail/descr.h(59): error: expression must have a constant value
      static constexpr auto digits = descr<sizeof...(Digits)>(('0' + Digits)...);

@henryiii
Copy link
Collaborator

This error is a regression and makes pybind11 master and 2.x unusable with CUDA. Adding the word detail:: fixes the issue

henryiii added a commit to henryiii/pybind11 that referenced this issue Oct 24, 2017
@jagerman
Copy link
Member

The original issue is fixed by 32ef69a which I just pushed. I suspect @loriab's issue is going to be more annoying to track down and fix (I think it was probably caused initially by #934).

@henryiii
Copy link
Collaborator

Did you push it to the 2.2 branch as well?

@jagerman jagerman added this to the v2.2.2 milestone Oct 24, 2017
@jagerman
Copy link
Member

No, but I'll set the milestone to remind us to include it in the 2.2.2 release.

@jagerman
Copy link
Member

@loriab - is that issue under master, or the 2.2 release? I don't think the #934 changes made it into 2.2.

@jagerman
Copy link
Member

(I only have ICC 17 available to me, where both of these compile okay).

@henryiii
Copy link
Collaborator

henryiii commented Oct 24, 2017

Okay, I'd like to have it in soon, because I've transitioned to 2.2.1 using a normal compiler, but it's now broken under CUDA, so switching the checkout to the 2.2 branch would be a temporary fix. I closed my PR since I assume it's best to cherry-pick the same commit.

By the way, just for general information, you can grab NVIDIA's docker containers and build with CUDA there in normal Docker, just (obviously) can't run.

(CUDA seems to be happy with the Digits line)

@loriab
Copy link
Contributor Author

loriab commented Oct 24, 2017

Thanks for the help. I just rebuilt with pybind11 master. Confirm that the original cast_op_type fixed for me in master and the next issue Digits remains.

@jagerman
Copy link
Member

Try playing around with that line to see if you can make ICC 16 happy (as I said, 17 is fine with it).

Does changing ('0' + Digits)... to ('0' + static_cast<char>(Digits))... help? (I'm basically just guessing right now).

@jagerman
Copy link
Member

Another possible solution is replace ('0' + Digits)... with digit_to_str(Digits)... and add:

inline constexpr char digit_to_str(size_t digit) { return static_cast<char>('0' + digit); }

just before the templated struct int_to_str.

@loriab
Copy link
Contributor Author

loriab commented Oct 24, 2017

Regrettably, icpc hates all your kind suggestions, as well as those variations my limited c++ imagination can devise. I'm giving up for the moment, but I'm glad to try more.

And while we like 2016 for some software, I'll accept it if you want to give up on 2016 support.

@loriab
Copy link
Contributor Author

loriab commented Nov 15, 2017

@bennybp, who actually understands templating in C++, took a look at this with 2016 icpc and, though not looking up any particular known error, concluded that it couldn't be fixed through ready code modifications. I've forced cmake_example to use pybind11 master and get the same digits error.

So, I suggest pybind11 officially drop 2016 support, codify it, and close this issue.

I can't pinpoint the fix point any nearer than >2016.3, <=2017.4, but if you want a PR with blanket Intel 15 --> 17 requirements, I can probably do that. Otherwise, feel free to close.

@jagerman
Copy link
Member

Personally I stick to all open source components for my projects as much as possible, which means I'm not at all familiar with the Intel compiler ecosystem: Is a 2017 minimum requirement here reasonable, or do ICC users tend to stick with older releases (e.g. Intel 15)?

It might be feasible to reintroduce the older implementation (perhaps stuffing it out into something like detail/icc.h) if upping the requirement would cause difficulties.

@loriab
Copy link
Contributor Author

loriab commented Nov 15, 2017

Upping the requirement is no hardship to my project (get Linux compilers from Intel's open-source developers program) and our users get pre-compiled or use gcc/clang. For academic labs that buy Intel compilers, I expect many do keep them around for a few years until compile failures prompt an upgrade. But the overlap of code dev, modern C++, and old compilers is probably low.

I'm not keen to download 4gb packages for the ~5 releases between 2016.3 and 2017.4, so I'm willing to call 2017 the cutoff.

@loriab
Copy link
Contributor Author

loriab commented Nov 15, 2017

Update, I know someone who'll try out 2017.0.2 tomorrow, so that'll be a better approximation to an outright 2017 cutoff.

@jturney
Copy link

jturney commented Nov 16, 2017

I can confirm that it works with Intel 2017.0.2. No compile/link issues and test cases pass with our project.

@jagerman jagerman modified the milestones: v2.2.2, v2.2.3 Feb 18, 2018
@loriab loriab mentioned this issue Apr 16, 2018
11 tasks
@wjakob
Copy link
Member

wjakob commented Apr 17, 2018

Can somebody comment on the status of this discussion? My understanding is as follows:

  • the first issue (detail:: namespace indicator) is fixed in master and can be pulled into the next patch release.
  • We will cut off support for Intel compiler <= 2016. In this case, it would be good if somebody could create a PR which updates the README/docs and #if __INTEL_COMPILER (line 49 in include/pybind11/detail/common.h). I am not sure what number to put there.

@loriab
Copy link
Contributor Author

loriab commented Apr 17, 2018

Done! See #1363.

loriab added a commit to loriab/pybind11 that referenced this issue Apr 26, 2018
@proteneer
Copy link

I'm still seeing template using cast_op_type = cast_op_type; on pybind 2.2.4 - did this go into master?

@loriab
Copy link
Contributor Author

loriab commented Feb 7, 2019

Yes, this went into release v2.2.3. Since then, there's at least #1649 where cast_op_type pops up.

@proteneer
Copy link

So why is that upon pip installing pybind (2.2.4) I'm still seeing the lack of detail::cast_op_type?

@loriab
Copy link
Contributor Author

loriab commented Feb 7, 2019

special header, perhaps?

@proteneer
Copy link

proteneer commented Feb 7, 2019

I'm confused. On master it's fixed:

https://github.com/pybind/pybind11/blob/master/include/pybind11/cast.h#L881

Clearly shows the detail:: prefix.

but none of the releases 2.2.3, 2.2.4 have that line included:

https://github.com/pybind/pybind11/blob/v2.2/include/pybind11/cast.h#L838

@loriab
Copy link
Contributor Author

loriab commented Feb 7, 2019

ah, the PR assoc. with this issue was only restricting Intel versions: https://github.com/pybind/pybind11/pull/1363/files . Some other PR (presumably unreleased from master) must have made the change you're looking at.

@bstaletic
Copy link
Collaborator

Perhaps it wasn't a PR at all: 32ef69a

@proteneer
Copy link

This isn't a big deal - since I've already refactored out the cuda specific out to a separate module. It was surprising to see the compiler failing despite it being in master. Any idea why this change wasn't cherry picked into 2.2?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

8 participants