[BUG] ICC cannot compile master and current release due to ICC's bug

[gnaggnoyil]

Issue description

Due to an ICC bug, current usage of pybind11::detail::all_of in SFINAE-context will be rejected by ICC.

When compiling the code below using linux64 icc, the following error message will be omitted:

$ icpc -o test -std=c++17 -isystem $PYBIND11_ROOT/include -isystem /usr/include/python3.8 test.cpp
test.cpp(17): error: no instance of function template "foo" matches the argument list
            argument types are: (const char *)
        foo(x);
        ^
test.cpp(11): note: this candidate was rejected because at least one template argument could not be deduced
  void foo(Args &&...){
       ^

compilation aborted for test.cpp (code 2)

Tested with current master and latest release(2.6.0) and the problem exists.

This problem blocks usage of object_api<Derived>::operator() in cast.h, which eventually makes it unable to use expressions such as .def(pybind11::init<T...>(pybind11::arg("foo")...).

Reproducible example code

#include <iostream>
#include <type_traits>

#include <pybind11/pytypes.h>

using namespace pybind11;
using namespace pybind11::detail;

template <typename ...Args,
        typename = std::enable_if_t<all_of<is_positional<Args>...>::value>>
void foo(Args &&...){
        std::cout << "bbb" << std::endl;
}

int main(){
        const char *x = "aaa";
        foo(x);
        return 0;
}

[YannickJadoul]

@gnaggnoyil, have you seen #2573? Is this related? If so, it might be in the process of getting fixed.

[gnaggnoyil]

@YannickJadoul I haven't seen any changes about cast.h nor types.h. Also I tried my example code using the branch in that PR and the problem still exists.

[YannickJadoul]

Do you have any idea what's different in your setup, then, compared to the CI being set up in #2573? Do you have a different version of ICC?

[gnaggnoyil]

@YannickJadoul I tried this example code in CentOS for icc 19.1.1 and 21.1.9, both of which reported the same error and blocked usage of object_api<Derived>::operator(). I don't have an Ubuntu 20.04 environment nor have idea about release version of icc's apt package, but Intel has already confirmed this bug in its compiler frontend thus the errors in OP should be constantly reproducible I think.

[tobiasleibner]

Do you have any idea what's different in your setup, then, compared to the CI being set up in #2573? Do you have a different version of ICC?

This is due to the std=c++17. I can confirm that this also happens in the tests in #2573 when using C++17. I encountered this before and already have a fix. However, the fix requires C++17, so it is probably cannot be merged here. However, it might be a workaround for @gnaggnoyil.

[tobiasleibner]

I tried compiling the tests with ICC and C++17, and there are several occurrences of this error. Not all of them can be fixed in the same way as the fix in my last comment, so we probably need a different workaround here.

[YannickJadoul]

@tobiasleibner, thanks, that's great! :-) Maybe with a bunch of preprocessor guards, we could fabricate something that works on both C++17 and pre-C++17? But then again, ICC is just wrong/broken, so it feels icky to fix and maintain such a fix in pybind11? :-/ But that's a discussion for #2573 and/or a future PR, I guess.

Dependently on how #2573 goes and whether you'd like to propose your solution as PR, should we then close this issue? @gnaggnoyil?

[tobiasleibner]

A fix for this should now be included in #2573. Of course, your reproducer will still fail to compile, but you should be able to fix it by using

template <typename ...Args>
constexpr bool all_args_are_positional()
{
  return all_of<is_positional<Args>...>::value;
}

template <typename ...Args,
        typename = std::enable_if_t<all_args_are_positional<Args...>()>>
void foo(Args &&...){
        std::cout << "bbb" << std::endl;
}

instead of

template <typename ...Args,
        typename = std::enable_if_t<all_of<is_positional<Args>...>::value>>
void foo(Args &&...){
        std::cout << "bbb" << std::endl;
}

[dmikushin]

#2714 seems to be exactly the same with NVIDIA HPC SDK compiler.

[gnaggnoyil]

@YannickJadoul The branch in @2753 hasn't yet fixed this issue now, but I indeed found the workaround in branch tobiasleibner/topic-icc and can confirm the error will no longer happen with this branch after transforming the reproducer like above. I also tried using expressions .def(pybind11::init<T...>(pybind11::arg("foo")...) and can confirm the usage is no longer blocked.

My intention of submitting this issue is just to notify the existence of this error thus as long as there are fixes/workarounds being proposed and/or users can be informed, I'm ok this issue to be closed.

[YannickJadoul]

We can see if #2573 will fix it, and leave it open for a bit longer :-)

[mkuron]

I have a minimal example that reproduces the bug with Intel ICC version 19.0.2 and 19.0.5:

#include <pybind11/pybind11.h>

void test2( pybind11::object obj ) {
   obj();
}

and compile via icpc -std=c++17 -c $(python3-config --includes) -I./include test.cpp. This gives me a long chain of errors that ends in

[...]
./include/pybind11/cast.h(2199): error: no instance of overloaded function "pybind11::detail::collect_arguments" matches the argument list
      return detail::collect_arguments<policy>(std::forward<Args>(args)...).call(derived().ptr());
             ^
./include/pybind11/cast.h(2185): note: this candidate was rejected because at least one template argument could not be deduced
  unpacking_collector<policy> collect_arguments(Args &&...args) {
                              ^
./include/pybind11/cast.h(2178): note: this candidate was rejected because at least one template argument could not be deduced
  simple_collector<policy> collect_arguments(Args &&...args) {
                           ^
          detected during instantiation of "pybind11::object pybind11::detail::object_api<Derived>::operator()(Args &&...) const [with Derived=pybind11::handle, policy=pybind11::return_value_policy::automatic_reference, Args=<>]" at line 4 of "test.cpp"

In C++14 mode, on the other hand, it compiles just fine. GCC has no problem compiling in either C++17 or C++14 mode. @dmikushin's patch in #2714 makes the issue go away. That patch appears to be incomplete though: I can still trigger a similar error message with

#include <pybind11/pybind11.h>

void test1() {
  pybind11::print("Hello World");
}

but at least it's now printed only once and not dozens of times like it is without the patch:

./include/pybind11/pybind11.h(2036): error: no instance of overloaded function "pybind11::detail::collect_arguments" matches the argument list
            argument types are: (const char [21])
      auto c = detail::collect_arguments<policy>(std::forward<Args>(args)...);
               ^
./include/pybind11/cast.h(2185): note: this candidate was rejected because at least one template argument could not be deduced
  unpacking_collector<policy> collect_arguments(Args &&...args) {
                              ^
./include/pybind11/cast.h(2178): note: this candidate was rejected because at least one template argument could not be deduced
  simple_collector<policy> collect_arguments(Args &&...args) {
                           ^
          detected during instantiation of "void pybind11::print(Args &&...) [with policy=pybind11::return_value_policy::automatic_reference, Args=<const char (&)[21]>]" at line 4 of "test.cpp"

@tobiasleibner, you said that #2573 contains a fix, but that's evidently not the case as it only changes CI and tests. Your patch from https://zivgitlab.uni-muenster.de/ag-ohlberger/dune-community/dune-xt/-/wikis/How-to-update-pybind11 does work, but it breaks C++14 compatibility because it uses if constexpr. If I naively replace that with an if, I get a different error but one that appears with all compilers and C++ standards.

[tobiasleibner]

@tobiasleibner, you said that #2573 contains a fix, but that's evidently not the case as it only changes CI and tests.

I thought the fix would be included in #2573, but apparently it will rather be included in a followup PR. You can find the fix in my topic-icc branch.

[mkuron]

Thanks, I have confirmed that https://github.com/tobiasleibner/pybind11/tree/topic-icc fixes these problems for me. I hope we can get that into pybind11 master soon!

There is a different problem now though that I have not found a minimal example for yet:

./pybind11/pybind11.h(1241): internal error: assertion failed: node_has_side_effects: bad node kind (shared/cfe/edgcpfe/il.c, line 23030)

          PYBIND11_EXPAND_SIDE_EFFECTS(add_base<options>(record));

I don't know if that's caused by your patch, @tobiasleibner, or maybe it only appeared now because it never got that far in compiling my project. I will investigate and open another issue if necessary.

[mkuron]

@tobiasleibner, here's a minimal example:

#include <pybind11/pybind11.h>

class C {
};

void test3(pybind11::module_ &m) {
	pybind11::class_<C>(m, "C");
}

Without your patch, ICC in C++17 mode produces those no instance of overloaded function errors. With your patch, it produces

./pybind11/pybind11.h(1241): internal error: assertion failed: node_has_side_effects: bad node kind (shared/cfe/edgcpfe/il.c, line 23030)

          PYBIND11_EXPAND_SIDE_EFFECTS(add_base<options>(record));

ICC in C++14 mode and GCC in either mode compiles just fine. To fix the bug, one can apply the following change:

--- a/include/pybind11/detail/common.h
+++ b/include/pybind11/detail/common.h
@@ -690,7 +690,7 @@ template <typename T> using is_lambda = satisfies_none_of<remove_reference_t<T>,
 inline void ignore_unused(const int *) { }
 
 /// Apply a function over each element of a parameter pack
-#ifdef __cpp_fold_expressions
+#if defined(__cpp_fold_expressions) && !defined(__INTEL_COMPILER)
 #define PYBIND11_EXPAND_SIDE_EFFECTS(PATTERN) (((PATTERN), void()), ...)
 #else
 using expand_side_effects = bool[];

Could you add that to your topic-icc branch or do you have a better suggestion how to solve this issue?

[tobiasleibner]

Could you add that to your topic-icc branch or do you have a better suggestion how to solve this issue?

Unfortunately, I most likely won't have time to work on this before the end of January, so feel free to copy my topic-icc branch, add your patch and open a PR (referencing this issue and #2573). Your patch looks reasonable to me, but I am not too familiar with the inner workings of pybind11, so maybe someone else will point out a better solution once you open that PR.

You might also want to add a CI run using icc and C++17 (simply copy the CI setup added here and change the -DCMAKE_CXX_STANDARD=11 line to -DCMAKE_CXX_STANDARD=17, I guess)

If you can't spare time, I will do this probably at the end of January/early February.