Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Intel C++ cannot convert `T **` to `T const * const *`, GCC can

Tags:

gcc

c++11

icc

The Problem

Extension of Existing Code

I have a numerical library that has been designed with one “flavor” in mind. Now I want to generalize this. The basic data structure is a “spinor” which itself is a multi dimensional matrix. There are lots of functions which take arrays of these spinors. The generalized functions need to take one such spinor array for each flavor.

Say there is a function which, minimally, does the following:

void copy_spinor(Spinor *out, const Spinor *in) {
    std::cout << out << " " << in << "\n";
}

My generalization now is this:

void copy_spinor(Spinor *out[num_flav], const Spinor *const in[num_flav]) {
    std::cout << "Fwd: ";
    copy_spinor(out[0], in[0]);
}

In the real code, there is a loop over all num_flav, but that is not really needed for this demonstration here.

As far as I understand, one has to read this as const Spinor *(in[num_flav]), so in is a pointer to an array of probably num_flav elements (or another quantity because foo[] is just *foo in a function parameter) of type pointer-to-const-spinor.

The problem is that it does not compile when using a Spinor *non_const[2] (without the const), see my earlier question. From the answer there I have learned that this must not compile because within the function copy_spinor, the pointer non_const[0] could be made to point to some const array of Spinor *. Then non_const would point to const data. Therefore that does not work.

My conclusion was that adding another const will make it correct:

void copy_spinor(Spinor *out[num_flav], const Spinor *const in[num_flav]) {}

When I now pass my non_const as the second parameter, the function cannot change in[0] to anything, because that pointer is now immutable. This has served me well with GCC 6.3. Now going into production with Intel C++ 17, it does not work any more.

The minimal working example is the following:

#include <cstdint>

typedef float Spinor[3][4][2][8];

template <uint8_t num_flav>
class Solver {
  public:
    void copy_spinor(Spinor *out, const Spinor *in) {
        std::cout << out << " " << in << "\n";
    }

    void copy_spinor(Spinor *out[num_flav], const Spinor *const in[num_flav]) {
        std::cout << "Fwd: ";
        copy_spinor(out[0], in[0]);
    }
};

int main(int argc, char **argv) {
    Spinor *s1 = new Spinor[10];
    Spinor *s2 = new Spinor[10];

    Spinor *s1_a[1] = {s1};
    Spinor *s2_a[1] = {s2};

    Solver<1> s;

    s.copy_spinor(s2_a, s1_a);
}

On GCC, it apparently resolves to the second copy_spinor overload. The variable s1_a which takes the role of the previous non_const, is allowed as an argument.

Problems with Intel C++ 17

Intel C++ 17 however, does not accept that:

$ icpc -Wall -pedantic const-spinor-const.cpp  --std=c++11
const-spinor-const.cpp(23): error: no instance of overloaded function "Solver<num_flav>::copy_spinor [with num_flav=(uint8_t={unsigned char})'\001']" matches the argument list
            argument types are: (Spinor *[1], Spinor *[1])
            object type is: Solver<(uint8_t)'\001'>
      s.copy_spinor(s2_a, s1_a);
        ^
const-spinor-const.cpp(11): note: this candidate was rejected because arguments do not match
      void copy_spinor(Spinor *out[num_flav], const Spinor *const in[num_flav]) {}
           ^
const-spinor-const.cpp(10): note: this candidate was rejected because arguments do not match
      void copy_spinor(Spinor *out, const Spinor *in) {}
           ^

The error message is not particularly helpful because it does not say what conversion was not allowed. It just seems that the const is the problem.

Is there perhaps something I miss with Intel C++? Is it lacking a feature or did I make use of an unofficial GCC extension? Is that a bug in Intel C++ or GCC?

Update: The example also compiles with current Clang.

Non-Template Class

The same issue also persists when the class Solver is not a template class. Since T a[2] is the same as T a[2] and T *a in a function argument, I can also just write the function like this, not needing the uint8_t num_flav:

void copy_spinor(Spinor *out[], const Spinor *const in[]) {
    std::cout << "Fwd: ";
    copy_spinor(out[0], in[0]);
}

The error is the same.

Free Functions

The same issue also happens for non-member non-friend non-template functions:

void free_spinor(Spinor *out, const Spinor *in) {
    std::cout << out << " " << in << "\n";
}

void free_spinor(Spinor *out[], const Spinor *const in[]) {
    std::cout << "Fwd: ";
    free_spinor(out[0], in[0]);
}

The error message is just the same:

$ icpc -Wall -pedantic const-spinor-const.cpp  --std=c++11
const-spinor-const.cpp(97): error: no instance of overloaded function "free_spinor" matches the argument list
            argument types are: (Spinor *[1], Spinor *[1])
      free_spinor(s2_a, s1_a);
      ^
const-spinor-const.cpp(30): note: this candidate was rejected because arguments do not match
  void free_spinor(Spinor *out[], const Spinor *const in[]) {
       ^
const-spinor-const.cpp(26): note: this candidate was rejected because arguments do not match
  void free_spinor(Spinor *out, const Spinor *in) {
       ^

Solution Attempts

In order to run the code in production, I see the following options. None of them are particularly attractive.

What would be a good way to go forward? I can alter my new functions all I want, but I would like to avoid touching caller code as much as possible.

Const Wrappers

When I change the definition of s1_a in the main function to have the two const, it compiles:

const Spinor *const s1_a[1] = {s1};

Then the function copy_spinor gets called with the correct argument type.

Every user of the generalized code has to write those const wrappers for every single function call. That will get really messy.

Remove Const Correctness.

I can remove the left-most const from the function argument parameters. That compiles cleanly on both compilers. However, I do want to document that I am not changing anything in that array, therefore its values should be const.

A partial solution would be to use some preprocessor constant that removes the const for the Intel Compiler only.

#ifdef __INTEL_COMPILER
#define ICPC_CONST
#else
#define ICPC_CONST const
#endif

Perhaps a user has defined some spinor as const. Then I am stuck and need to have the const there in place:

const Spinor *s3_a[1] = {s3};

s.copy_spinor(s2_a, s3_a);

It should be easier to add a const than to remove it, so this solution is pretty lacking. Also the upstream author would likely reject my changes due to the changes in his code.

Add a non-const Overload for each Function

Adding an overload for each function would be possible. I have two variants of the generalized function, the second one gets enabled when I work with the Intel compiler:

void copy_spinor(Spinor *out, const Spinor *in) {
    std::cout << out << " " << in << "\n";
}

void copy_spinor(Spinor *out[num_flav], const Spinor *const in[num_flav]) {
    std::cout << "Fwd: ";
    copy_spinor(out[0], in[0]);
}

#ifdef __INTEL_COMPILER
void copy_spinor(Spinor *out[num_flav], Spinor *const in[num_flav]) {
    std::cout << "Fwd2: ";
    copy_spinor(out[0], in[0]);
}
#endif

That works well, there is just some duplication of code. Since my added functions just re-use the existing functions, it is not that much code duplication. Still a violation of the DRY principle.

Another disadvantage is that the number of overloads is 2^N where N is the number of parameters that are a const *. There are functions taking up to three arguments like this, therefore I would need eight copies.

Let the Template Deduce the Constness

Abstracting const Spinor and Spinor away is possible with templates. I could write the function as a template such that S can be either data type. Using a static_assert will give a slightly more informative error message.

template <typename S>
void copy_spinor(Spinor *out[num_flav], S *const in[num_flav]) {
    static_assert(std::is_same<Spinor, S>::value ||
                      std::is_same<const Spinor, S>::value,
                  "Template parameter must be `const Spinor` or `Spinor`.");

    std::cout << "Fwd: ";
    copy_spinor(out[0], in[0]);
}

Ideally, I would like to specify that S can only be a Spinor or const Spinor. Perhaps that is possible with C++14 and beyond, I have to stick with C++11.

This solution looks pretty clean, I can add a template argument and an assert for each argument in the function. This will scale pretty good and there is no code duplication. The only downside could be longer compilation time (already rather long, but not really important) and less useful error messages (hopefully covered with the static_assert).

The error message when calling it with int ** is the following for GCC:

const-spinor-const.cpp: In instantiation of 'void Solver<num_flav>::t_copy_spinor(float (**)[3][4][2][8], S* const*) [with S = int; unsigned char num_flav = 1u; Spinor = float [3][4][2][8]]':
const-spinor-const.cpp:86:36:   required from here
const-spinor-const.cpp:40:9: error: static assertion failed: Template parameter must be `const Spinor` or `Spinor`.
         static_assert(std::is_same<Spinor, S>::value ||
         ^~~~~~~~~~~~~
const-spinor-const.cpp:45:20: error: no matching function for call to 'Solver<1u>::copy_spinor(float (*&)[3][4][2][8], int* const&)'
         copy_spinor(out[0], in[0]);
         ~~~~~~~~~~~^~~~~~~~~~~~~~~
const-spinor-const.cpp:29:10: note: candidate: void Solver<num_flav>::copy_spinor(float (*)[3][4][2][8], const float (*)[3][4][2][8]) [with unsigned char num_flav = 1u; Spinor = float [3][4][2][8]]
     void copy_spinor(Spinor *out, const Spinor *in) {
          ^~~~~~~~~~~
const-spinor-const.cpp:29:10: note:   no known conversion for argument 2 from 'int* const' to 'const float (*)[3][4][2][8]'
const-spinor-const.cpp:33:10: note: candidate: void Solver<num_flav>::copy_spinor(float (**)[3][4][2][8], const float (* const*)[3][4][2][8]) [with unsigned char num_flav = 1u; Spinor = float [3][4][2][8]]
     void copy_spinor(Spinor *out[num_flav], const Spinor *const in[num_flav]) {
          ^~~~~~~~~~~
const-spinor-const.cpp:33:10: note:   no known conversion for argument 1 from 'float (*)[3][4][2][8]' to 'float (**)[3][4][2][8]'

In the comments it was pointed out to use enable_if. With that, my function looks like the following:

template <typename S>
typename std::enable_if<std::is_same<const Spinor, const S>::value,
                        void>::type
t2_copy_spinor(Spinor *out[num_flav], S *const in[num_flav]) {
    std::cout << "Fwd: " << typeid(S).name() << " " << typeName<S>() << " ";
    copy_spinor(out[0], in[0]);
}

This is shorter, and perhaps more succint. The error message does not contain my hand-written message any more , though. At least the error does not occur within the function copy_spinor but at the calling site, so the user knows where it went wrong. That is perhaps better. And the enable_if somewhat explains itself, at least to more experienced template users.

const-spinor-const.cpp: In function 'int main(int, char**)':
const-spinor-const.cpp:86:37: error: no matching function for call to 'Solver<1u>::t2_copy_spinor(float (* [1])[3][4][2][8], int* [2])'
     s.t2_copy_spinor(s2_a, int_array);
                                     ^
const-spinor-const.cpp:51:5: note: candidate: template<class S> typename std::enable_if<std::is_same<const float [3][4][2][8], const S>::value, void>::type Solver<num_flav>::t2_copy_spinor(float (**)[3][4][2][8], S* const*) [with S = S; unsigned char num_flav = 1u]
     t2_copy_spinor(Spinor *out[num_flav], S *const in[num_flav]) {
     ^~~~~~~~~~~~~~
const-spinor-const.cpp:51:5: note:   template argument deduction/substitution failed:
const-spinor-const.cpp: In substitution of 'template<class S> typename std::enable_if<std::is_same<const float [3][4][2][8], const S>::value, void>::type Solver<num_flav>::t2_copy_spinor(float (**)[3][4][2][8], S* const*) [with S = int]':
const-spinor-const.cpp:86:37:   required from here
const-spinor-const.cpp:51:5: error: no type named 'type' in 'struct std::enable_if<false, void>'

The enable_if solution looks better than the static_assert variant.

like image 259
Martin Ueding Avatar asked Mar 28 '17 09:03

Martin Ueding


1 Answers

GCC and clang are right here, Intel C++ is wrong.

The relevant portion of the Standard is Qualification conversions [conv.qual] (the section number may be either 4.4 or 4.5). The wording has changed between C++11 and C++1z (to become C++17)... but your code which adds const at multiple levels starting with the shallowest, is allowed in all versions (C++03, 11, 14, 1z).

One change of note is that the multi-level const rule now applies to arrays of pointers, where formerly it only applied to multiple pointers. But we actually are dealing with a multiple pointer, because the array syntax, when found in a parameter declaration, has pointer semantics.

Still, it might be worthwhile to try

void copy_spinor(Spinor **out, const Spinor *const *in)

in case the compiler got confused / failed to adjust array types to pointer types within a function parameter list before applying the rule.

like image 155
Ben Voigt Avatar answered Nov 16 '22 17:11

Ben Voigt