Skip to content

Conversation

@jvdp1
Copy link
Member

@jvdp1 jvdp1 commented Oct 30, 2025

Based on #1031, #1033
Closes #1049

Here is a proposition to modularize stdlib using cpp directives (instead of fypp) based on this comment

\cc: @eduardz1 @jalvesz @perazz

@jvdp1
Copy link
Member Author

jvdp1 commented Oct 30, 2025

here is a proposal (to replace #1049) that relies only on the CPP directives.
The code using bitsets is always generated and encapsulated between the cpp directive STDLIB_BITSET.

@jalvesz : I didn't look carefully to fypp_deployment.py as it results in many errors on my computer. If we are all happy, I can have a look later.

@jvdp1 jvdp1 requested review from jalvesz and perazz October 30, 2025 20:25
@jalvesz
Copy link
Contributor

jalvesz commented Nov 1, 2025

Thanks for this new approach @jvdp1. One question though, I didn't see the header file defining the default value of the internal define parameter. If so, one would be obliged to pass a flag even when building with a compiler that does support it, right?

@jvdp1
Copy link
Member Author

jvdp1 commented Nov 1, 2025

One question though, I didn't see the header file defining the default value of the internal define parameter. If so, one would be obliged to pass a flag even when building with a compiler that does support it, right?

Sorry I forgot about it. I will add one, such that the default is the compilation of bitsets.

@jvdp1
Copy link
Member Author

jvdp1 commented Nov 1, 2025

@jalvesz : I added a file macros.inc, such that the default is the compilation of the whole stdlib (i.e., including stdlib_bitsets. The user must pass the flag STDLIB_NO_BITSET to avoid its compilation.

However, I think that the file macros.inc is not required if the directive
#ifdef STDLIB_BITSET
is replaced by
#ifndef STDLIB_NO_BITSET

@codecov
Copy link

codecov bot commented Nov 3, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 25.13%. Comparing base (fffe0d7) to head (ad9798f).

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1050      +/-   ##
==========================================
+ Coverage   25.12%   25.13%   +0.01%     
==========================================
  Files         570      570              
  Lines      234201   234201              
  Branches    41277    41267      -10     
==========================================
+ Hits        58838    58865      +27     
+ Misses     175363   175336      -27     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@jalvesz
Copy link
Contributor

jalvesz commented Nov 12, 2025

Sorry @jvdp1, I was out of office for a while and couldn't check everything before leaving.

However, I think that the file macros.inc is not required if the directive
#ifdef STDLIB_BITSET
is replaced by
#ifndef STDLIB_NO_BITSET

You are absolutely write about this. What would you prefer to stick with? What I do find better for readability is formulating in "positive" terms. Also, while not mandatory, if we stick with using the include approach, it would be maybe good to have it explicitly indicating the values:

#ifdef STDLIB_NO_BITSET
#define STDLIB_BITSET 0
#else
#define STDLIB_BITSET 1
#endif

Which would lead to the use of #if STDLIB_BITSET ... ; #else ... ; #endif

@jvdp1
Copy link
Member Author

jvdp1 commented Nov 12, 2025

Sorry @jvdp1, I was out of office for a while and couldn't check everything before leaving.

No problems.

However, I think that the file macros.inc is not required if the directive
#ifdef STDLIB_BITSET
is replaced by
#ifndef STDLIB_NO_BITSET

You are absolutely write about this. What would you prefer to stick with? What I do find better for readability is formulating in "positive" terms.

You are right: "positive" terms may improve readaiblity. As it is an internal approach, we can keep it like this, and if needed, we can revise it later.

Also, while not mandatory, if we stick with using the include approach, it would be maybe good to have it explicitly indicating the values:

#ifdef STDLIB_NO_BITSET
#define STDLIB_BITSET 0
#else
#define STDLIB_BITSET 1
#endif

Which would lead to the use of #if STDLIB_BITSET ... ; #else ... ; #endif

OK. This approach is a bit more verbose (#if STDLIB_BITSET == 1 instead of #ifdef STDLIB_BITSET) but it could be more readable and flexible. I will give it a try.

@jvdp1
Copy link
Member Author

jvdp1 commented Nov 12, 2025

@jalvesz I implemented your suggestions. It is indeed more readable like this. Let me think what you think about this approach. If it is ok, it can be merged, and I will start implementing this approach for another module.

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