VNL is not C++17 compatible

Engineering
1

Hello,

I’ve just tried building ITK master with C++17 enabled with Apple’s latest compiler Apple LLVM version 10.0.0 (clang-1000.11.45.2) and I get the following error:

In file included from ../Modules/ThirdParty/VNL/src/vxl/v3p/netlib/v3p_netlib.h:5:
../Modules/ThirdParty/VNL/src/vxl/v3p/netlib/v3p_f2c.h:29:14: error: ISO C++17 does not allow 'register' storage class specifier [-Wregister]
void sig_die(register char *s, int kill);

I’ll try compiling ITK with a lower C++ standard, but that has produced warnings for me before. I am aware that ITK has only just made it onto C++11, but when I tried if constexpr it simplified a class of mine so much I decided it was worth sticking with.

2

Since it is a warning, we could suppress it for now (this is just C code).

And start thinking more about moving from VNL to Eigen for ITK 6…

3

I wish I had the time to volunteer for that!

1 Like
4

Matt,

It’s not a warning, it’s an error. ‘register’ was deprecated previously, but is entirely removed in C++17.

Sean

1 Like
5

:frowning:

@spinicist do you have more information on the traceback and the command that is invoked? Can we do the compilation as C instead of C++?

6

As register keyword was an optimization hint since it was introduced, just removing it from the code is probably the best solution? Of course, submit the patch to VNL and then update ITK’s copy of it.

7

Yup, that’s the only solution if VNL is a codebase that wants to keep compiling in the future. :slight_smile:

8

error: ISO C++17 does not allow ‘register’ storage class specifier [-Wregister]

What does -Wregister (-Wno-register) argument do? May be it turns error into warning. But anyway removing “register” from code is good.

9

@matt.mccormick There wasn’t anything special about this compilation - I configured with -DCMAKE_CXX_STANDARD=17 and then compiled. Apple updated Clang last week with better C++17 compliance.

As I can make this work by dropping back to C++14/11 to compile ITK, I don’t really want to go through the process of submitting a patch to VNL. If I spent anytime in that direction, I’d prefer it to be on getting Eigen into ITK.

10

I think we agreed we wanted Eigen to replace VNL. But I am afraid we estimated that to be quite an effort. And minor cleanup of VNL to be C++17 compatible is a sneeze compared to a switch to Eigen.

11

I went ahead and created a PR to vxl here:

CC: @hjmjohnson

3 Likes
12

Thanks @matt.mccormick

1 Like