Skip to content

Conversation

@jvdp1
Copy link
Member

@jvdp1 jvdp1 commented Oct 24, 2025

Based on #1031, #1033

Here is a proposition to modularize stdlib_bitsets in cmake.
Furthermore, compiling stdlib_bitsets can be now set off with cmake

\cc: @eduardz1 @jalvesz @perazz

@jvdp1 jvdp1 requested review from jalvesz and perazz October 24, 2025 10:35
@jalvesz
Copy link
Contributor

jalvesz commented Oct 24, 2025

I would have say to better rely on C-preprocessing and passing a flag definition -DWITH_BITSET instead of fypp prepreprocessing. That way the same source files can be processed by different compilers generating downstream different binaries with the appropriate flags.

@jvdp1
Copy link
Member Author

jvdp1 commented Oct 24, 2025

I would have say to better rely on C-preprocessing and passing a flag definition -DWITH_BITSET instead of fypp prepreprocessing. That way the same source files can be processed by different compilers generating downstream different binaries with the appropriate flags.

I agree, but I could not find a backward-compatible approach with C-preprocessing.
To be backward-compatible, the user that does not provide a flag definition will compile the whole library.
With fypp, we can set the flag WITH_BITSET to true per default, and the user does not need to define the flag (except if the user does not want to compile the module bitsets).

However, with cpp, something like #ifdef _WITH_BITSETS will require that the user provides the flag to compile the module bitsets.
I though about the cpp directive #ifndef _WITHOUT_BITSETS, but I did not find it elegant and counter-intuitive.

Any other suggestions?

@jalvesz
Copy link
Contributor

jalvesz commented Oct 28, 2025

Here is an idea:
In the include or src folder we add a file macros.h. Within, it can be defined:

#ifdef STDLIB_WITH_BITSET
#define WITH_BITSET 1
#else
#define WITH_BITSET 0
#endif

This file can be included elsewhere with #include 'macros.h' to use #ifdef WITH_BITSET. Like this, a default can be defined within and the user can override if needed with a flag -DSTDLIB_WITH_BITSET (or the opposite, depending which default to set).

@jvdp1
Copy link
Member Author

jvdp1 commented Oct 28, 2025

Here is an idea: In the include or src folder we add a file macros.h. Within, it can be defined:

#ifdef STDLIB_WITH_BITSET
#define WITH_BITSET 1
#else
#define WITH_BITSET 0
#endif

This file can be included elsewhere with #include 'macros.h' to use #ifdef WITH_BITSET. Like this, a default can be defined within and the user can override if needed with a flag -DSTDLIB_WITH_BITSET (or the opposite, depending which default to set).

Thank you. I tried with this strategy, but then I was challenged with the fypp loop approach, such as:

    #:for k1, t1 in IRB_KIND_TYPES
      module procedure :: swap_${k1}$
   #:endfor

With fypp, it is easy to exclude or include the support for DT bitset_type when defining the variable IRB_KIND_TYPES. However, it is not so straightforward with the combinasion of fypp and cpp.

An ugly (at least for me) solution could be:

    #:for k1, t1 in IRB_KIND_TYPES
    #:if 'bitset' in t1
#ifdef WITH_BITSET
    #:endif
      module procedure :: swap_${k1}$
    #:if 'bitset' in t1
#end
    #:endif
   #:endfor

I have the feeling that such an approach will result in a code even more difficult to follow and maintain.

All suggestions are welcome.

@jalvesz
Copy link
Contributor

jalvesz commented Oct 29, 2025

I tried to give it some more thoughts but indeed I can't think of a more elegant way of handling this. Your solution is very good, just wanted to check if we could rely on c-preprocessing which is much more standard.

In any case, I would say, given that both GNU and Intel compilers can handle all the features and are the most common compilers, to keep it activated by default and let users remove it with fypp for other compilers which can't compile these modules.

I would like to keep the idea of a macro header file open for future ideas though, I think it might facilitate many future hurdles.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants