Master branch build failure vs2017

Development
1

I’m seeing a compile error with vs2017, which is still a supported compiler according to the VTK build instructions:

  vtkTableBasedClipCases.cxx
  Internal Compiler Error in C:\Program Files (x86)\Microsoft Visual Studio\2017\Community\VC\Tools\MSVC\14.16.27023\bin\HostX86\x64\CL.exe.

The error doesn’t give much useful information, but I’ve tracked down the problem to the large arrays in the vtkTableBasedClipCases.h header file, e.g.:

  static constexpr uint8_t CellCases[] = {
    <26665 elements>
  };

It seems that, for vs2017, this array is just too large to be constexpr and basically crashes the compiler. As a test, I reduced the size to just a few hundred elements (and similarly reduced other arrays in this header), and the compilation worked just fine.

Would it be reasonable to put the few really huge arrays into the .cxx file, instead of keeping them in the .h file? It would be easier on the compiler, and probably wouldn’t impact performance much.

Edit: it’s probably just two arrays that would have to be moved: CellCases[26665] and CellCasesInsideOut[23879].

Edit 2: I’ve confirmed that vs2017 builds after these arrays are moved to the .cxx and declared “const” instead of “constexpr”.

2

FYI @ben.boeckel

3

Seems like a reasonable change to me. Note that we usually have data arrays like this in separate files anyways because clang-format has historically taken way too long to reformat them. See vtkEarthSource.cxx and vtkEarthSourceData.inl.

4

After pulling from master, I found another vs2017 compile failure, this one is due to the recently updated ThirdParty/exodusII package. The file ex_field_utils.c uses restrict, and vs2017 reports a syntax error as a result. Here is the line:

size_t my_strlcat(char *restrict dst,
                  const char *restrict src,
                  size_t maxlen)

Not sure why windows CI doesn’t catch this, perhaps vs2022 is okay with restrict?

5

Yeah, VS2022 is probably OK with it. Given the C++17 bump that is pending, we should probably add a mindeps Windows build as well.