Conflicting definition of IndexType

[nselliott]

In src/axom/core/Types.hpp we have:

 #if defined(AXOM_USE_64BIT_INDEXTYPE) && !defined(AXOM_NO_INT64_T)
 using IndexType = int64;
 #else
 using IndexType = int32;
 #endif

And in src/axom/sidre/core/SidreTypes.hpp we have:

 typedef SIDRE_IndexType IndexType;

SIDRE_IndexType is elsewhere defined as int64_t. We should define IndexType in only one place.

[gzagaris]

I think this could be probably fixed by adding the cpp_if in the sidre_shroud.yaml (hopefully) or we need to do something like that. We are using that in quest for conditionally compiling routines that take an MPI communicator, but, I am not sure if we can use it out-of-the-box for types.

@ltaylor16 thoughts?

[kennyweiss]

Thanks @nselliott.

The problem is actually here:
https://github.com/LLNL/axom/blob/7c1185420de47bbd4ccf466b9c5f1e38fd69891d/src/axom/sidre/core/SidreDataTypeIds.h#L23-L25

If we still need SIDRE_IndexType, I think it should be defined as follows:

using SIDRE_IndexType = axom::IndexType;

Since axom::IndexType defaults to 32 bits, and this one redefines it to 64 bits, this definitely needs to be resolved.

[kennyweiss]

Here are the relevant lines in Sidre's shroud:
https://github.com/LLNL/axom/blob/7c1185420de47bbd4ccf466b9c5f1e38fd69891d/src/axom/sidre/interface/sidre_shroud.yaml#L30-L35

@ltaylor16 -- could this line be changed to support a configuration dependent type (e.g. int32_t or int64_t) based on AXOM_USE_64BIT_INDEXTYPE?

[gzagaris]

Hmm....I am surprised we are hardcoding SIDRE_IndexType ourselves. It looks like we are hard-coding it to int64_t and then also aliasing sidre::IndexType to that.

I thought that shroud shroud would emit the definition of SIDRE_IndexType. I understand this was done a while ago, but, is there a reason shroud couldn't do this?

For example, based on how we do something similar in quest could we do something like this:

declarations:
   -decl: #define SIDRE_IndexType int64_t
     fields:
          c_header: axom/sidre/interface/SidreTypes.hpp
          cpp_if: ifdef AXOM_USE_64BIT_INDEXTYPE
   -decl: #define SIDRE_IndexType int32_t
     fields:
          c_header: axom/sidre/interface/SidreTypes.hpp
          cpp_if: ifndef AXOM_USE_64BIT_INDEXTYPE

[kennyweiss]

This issue appears to still be outstanding.

@ltaylor16 -- it seems like this is only necessary for Sidre's Fortran interface.
Can shroud handle a configurable SIDRE_IndexType to allow us to set SIDRE_IndexType to core::IndexType, which can either be an int32 or int64``?

EDIT: If not, do you see a way around this?

[rhornung67]

See: #194

[rhornung67]

We also have declarations like this sprinkled around in C++ code: https://github.com/LLNL/axom/blob/develop/src/axom/sidre/core/Buffer.hpp#L123
then we do this in the associated shroud layer: https://github.com/LLNL/axom/blob/develop/src/axom/sidre/interface/sidre_shroud.yaml#L473

Specifically, the C++ code is parameterized via a macro variable, while the yaml file has the type hardcoded.

I think we can make the C++ usage consistent without having a Sidre-specific index type (i.e., use a centrally-defined index type in Axom core), but I'm not sure what we can do to support Fortran via shroud (as has been discussed in this conversation previously).

@ltaylor16 What is the semantic meaning of the definition of IndexType vs. SIDRE_IndexType here? https://github.com/LLNL/axom/blob/develop/src/axom/sidre/interface/sidre_shroud.yaml#L36 Our API annotated in the yaml file contains 'IndexType' everywhere. Then this gets replaced by 'SIDRE_IndexType' in the generated C and Fortran interface files. Is it possible to have Shroud generate an interface for int32_t and another for int64_t and have each guarded so the right one gets used based on the compile time specified value of AXOM_USE_64BIT_INDEXTYPE? I believe this is what @kennyweiss @gzagaris were asking about earlier.

[ltaylor16]

The Fortran module should create a parameter of the integer type using a splicer. C_INT32_T and C_INT64_T are both part of the intrinsic module iso_c_bindings.

  #if defined(AXOM_USE_64BIT_INDEXTYPE)
  integer, parameter :: INDEXTYPE = C_INT64_T
  #else
  integer, parameter :: INDEXTYPE = C_INT32_T
  #endif

The C wrapper will also need a splicer

  #if defined(AXOM_USE_64BIT_INDEXTYPE)
  typedef int64_t IndexType;
  #else
  typedef int32_t IndexType;
  #endif

Then allow a typemap in the YAML file which defines how C++ IndexType should be expanded so that the C++ argument

IndexType arg

becomes

integer(INDEXTYPE) arg

Creating a custom typemap for an integer type in the YAML file requires some changes to shroud.