GmshIO plugin assumes signed char which breaks on ARM
At https://gitlab.kitware.com/paraview/paraview/-/blob/master/Plugins/GmshIO/IO/gmshCommon.h#L9 the GmshIO plugin sets
enum class GmshPrimitive : char
{
Unsupported = -1,
...
}
So a value of -1
is being set to type char
. This assumes that char is signed, which apparently is not always the case.
For instance on ARM (arm64) architecture, paraview 5.11.2 fails to build with GmshIO enabled, giving an error
[ 26%] Building CXX object VTK/Filters/Core/CMakeFiles/FiltersCore.dir/vtkMassProperties.cxx.o
/usr/bin/c++ -o vtkMassProperties.cxx.o -c vtkMassProperties.cxx
In file included from Plugins/GmshIO/IO/vtkGmshReader.cxx:16:
Plugins/GmshIO/IO/gmshCommon.h:9:18: error: enumerator value ‘-1’ is outside the range of underlying type ‘char’
9 | Unsupported = -1,
| ^
A full build log can be found at https://buildd.debian.org/status/fetch.php?pkg=paraview&arch=arm64&ver=5.11.2%2Bdfsg-3&stamp=1701895041&raw=0
At https://gitlab.kitware.com/paraview/paraview/-/blob/master/Plugins/GmshIO/IO/vtkGmshWriter.cxx#L55 GmshPrimitive
is used in association with VTKCellType as unsigned char
,
static const std::unordered_map<unsigned char, GmshPrimitive> TRANSLATE_CELLS_TYPE;
But vtkCellType.h does define VTK_EMPTY_CELL = 0
, so it probably would not work well to set GmshPrimitive::Unsupported
to 0 instead of -1.
The value (and sign) is used at https://gitlab.kitware.com/paraview/paraview/-/blob/master/Plugins/GmshIO/IO/vtkGmshWriter.cxx#L140 but handled alongside explicit unsigned char
,
for (unsigned char currentType = 1; currentType < GmshWriterInternal::MAX_TAG; ++currentType)
{
std::vector<std::size_t>& indexes = idxPerType[currentType];
if (indexes.empty())
{
continue;
}
char gmshType =
static_cast<char>(GmshWriterInternal::TRANSLATE_CELLS_TYPE.find(currentType)->second);
// If this type is not natively supported, it will be transleted in either lines or triangles
if (gmshType < 0)
{
continue;
}
...
So unsigned char
is explicitly used here for currentType
, while an unsupported type is handled via a negative value in gmshType
.
Should the GmshPrimitive enum in gmshCommon.h, and gmshType
in vtkGmshWriter.cxx, and elsewhere, be declared explicitly as type signed char
instead of char
? I gather that just using int
itself would not be appropriate.