Skip to content

Conversation

@khwilliamson
Copy link
Contributor

There are internal operations that use the widest integer type already. So fa,r not knowing its actual width at compile time has been worked around, but I'm planning enhancements that do need that value

And now that we require C99, the intmax_t type should always be available.

  • This set of changes does not require a perldelta entry.

@jkeenan
Copy link
Contributor

jkeenan commented Dec 28, 2025

Dies during make here:

cc -c -DPERL_CORE -fwrapv -fno-strict-aliasing -pipe -fstack-protector-strong -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 -O2 -Wall -Wno-use-after-free deb.c
In file included from perl.h:46,
                 from perl.c:39:
config.h:3820:3: error: invalid preprocessing directive #I_STDINT
 3820 | # I_STDINT              /**/
      |   ^~~~~~~~
In file included from perl.h:46,
                 from op.c:163:
config.h:3820:3: error: invalid preprocessing directive #I_STDINT
 3820 | # I_STDINT              /**/
      |   ^~~~~~~~
In file included from perl.h:46,
                 from deb.c:25:
config.h:3820:3: error: invalid preprocessing directive #I_STDINT
 3820 | # I_STDINT              /**/
      |   ^~~~~~~~
In file included from perl.h:46,
                 from caretx.c:32:
config.h:3820:3: error: invalid preprocessing directive #I_STDINT
 3820 | # I_STDINT              /**/
      |   ^~~~~~~~
In file included from perl.h:46,
                 from class.c:16:
config.h:3820:3: error: invalid preprocessing directive #I_STDINT
 3820 | # I_STDINT              /**/
      |   ^~~~~~~~
In file included from perl.h:46,
                 from universal.c:31:
config.h:3820:3: error: invalid preprocessing directive #I_STDINT
 3820 | # I_STDINT              /**/
      |   ^~~~~~~~
In file included from perl.h:46,
                 from av.c:20:
config.h:3820:3: error: invalid preprocessing directive #I_STDINT
 3820 | # I_STDINT              /**/
      |   ^~~~~~~~
In file included from perl.h:46,
                 from builtin.c:16:
config.h:3820:3: error: invalid preprocessing directive #I_STDINT
 3820 | # I_STDINT              /**/
      |   ^~~~~~~~
make: *** [makefile:265: deb.o] Error 1
make: *** Waiting for unfinished jobs....
make: *** [makefile:265: class.o] Error 1
make: *** [makefile:326: perlmini.o] Error 1
make: *** [makefile:265: caretx.o] Error 1
make: *** [makefile:265: builtin.o] Error 1
make: *** [makefile:265: av.o] Error 1
make: *** [makefile:334: universalmini.o] Error 1
make: *** [makefile:318: opmini.o] Error 1

@khwilliamson khwilliamson force-pushed the metaconfig branch 2 times, most recently from d318d48 to f4bb145 Compare December 31, 2025 20:15
Copy link
Contributor

@tonycoz tonycoz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks reasonable, but we do need to be careful when exposing intmax_t as part of the ABI.

Potentially compiler and/or libc updates, and compiler flags can change the size of intmax_t

Now that we require C99, we should be able to probe for this

Some internal operations can benefit from using the widest type possible
on the system, even if IVs and UVs don't use the full width.

This data is kept in <stdint.h>, which should always be available in
C99-compliant build systems.  Configure has a 'i_stdint', but it comes
much later in Configure than this does.  I tried moving it to earlier,
but then it relies on stuff that would have to get moved too, and I
think that stuff has to be later.

It gets complicated trying to deal with this value not existing, as
there is a separate unit for 'long long', which would have to be
considered.  Until and unless some platform comes along that this
doesn't work for, I just assume that this C99 construct will work as
expected.
@khwilliamson
Copy link
Contributor Author

@tonycoz regen/embed.pl could add checks to prevent a function declared as returning intmax_t from being visible outside the core, and maybe extensions. Should this apply to things like int_fast8_t as well?

This commit effectively only adds the ability to use the size of this in cpp #if statements. We already can use this typedef in ways that you caution about, so this commit adds no new bad outcomes that didn't already exist.

And, FWIW, we already have public functions that work fine, and take parameters like this. Perl_variant_byte_number(PERL_UINTMAX_T word) is an example

@tonycoz
Copy link
Contributor

tonycoz commented Jan 5, 2026

And, FWIW, we already have public functions that work fine, and take parameters like this. Perl_variant_byte_number(PERL_UINTMAX_T word) is an example

The problem isn't whether it works now, the problem is whether a compiler or libc upgrade or compiler option will change the size of intmax_t in the future. (Also, it isn't necessarily the largest integer type)

variant_byte_number() should be fairly safe, since it's static inline (it has no ABI), but for a function or type that's part of the ABI (parameter or return type of a function, member of a public struct) it's risky.

Annoying as it is it might be better to probe for a large integer type to use for such cases to ensure we have a stable ABI.

But I'm just saying be careful.

Should this apply to things like int_fast8_t as well?

No, there's no reason for the compiler (or libc) to change the size of such types.

Though I don't see much reason to use them for simple local variables or parameters, they save a tiny amount of stack space or none at all if the variable is allocated to a register.

They can be useful in structs or arrays, the uses in next_matchable_info are good.

Also, we can't be sure how the balance between speed and size were resolved, per the footnote in C23:

306)The designated type is not guaranteed to be fastest for all purposes; if the implementation has no clear grounds for
choosing one type over another, it will simply pick some integer type satisfying the signedness and width requirements.

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.

3 participants