util.c: mark internal symbols 'static' - #24137
Conversation
It was already declared 'static' in proto.h/embed.fnc, but for consistency, make the definition in util.c 'static' as well.
This is purely an internal helper function.
These are internal variables not intended to be part of our API, so make them static.
| * Examples: https://rosettacode.org/wiki/Pseudo-random_numbers/Splitmix64 | ||
| */ | ||
| U64 | ||
| static U64 |
There was a problem hiding this comment.
PR #24105 uses it, so it may need to be public.
Though I think it should be flagged "C" rather than "A".
That PR puts everything in a header and has similar visibility problems to this function.
There was a problem hiding this comment.
Technically not a problem because the proposed prng.h file (which is not really a header) is only getting included in util.c anyway, so it is still the same translation unit.
There was a problem hiding this comment.
Why not uppercase STATIC?
There was a problem hiding this comment.
Why randomly capitalize language keywords?
There was a problem hiding this comment.
For consistency. Maybe it's no longer a concern, in which case should we change all STATICs?
There was a problem hiding this comment.
To be honest, I don't know why STATIC exists. It is hardcoded in perl.h as #define STATIC static. We already have many uses of plain static in core (a naive rg '^\s*static\b' *.c *.h | wc -l reports 615 matches), so there is no consistency to be had anyway.
...digging...
The first occurrence of STATIC I can find is in perl-2.0's regexp.c (where it already expands to plain static). In perl-3.000 its definition was copied into both regcomp.c and regexec.c, and the former moved to regcomp_internal.h in 85900e2. These definitions still exist.
All non-regex uses of STATIC began in commit 76e3520, which changed some (but not all) occurrences of static to STATIC and added a conditional definition to perl.h: If PERL_OBJECT is defined, STATIC expands to nothing instead. PERL_OBJECT (a feature to let you define an interpreter as a C++ class CPerlObj, not a C struct or a collection of global variables) was removed in acfe0ab (5.7.x), but for some reason the STATIC declarations were not changed back to static. Since then, STATIC has always been hardcoded as static.
So to answer your questions:
- Under
PERL_OBJECT, makingsplitmix64STATICwould not have made sense: It does not need access to any member functions or variables ofCPerlObj. - Yes, all uses of
STATICin the core should have been ripped out in 5.8.
3 participants
As a general rule, our symbols are either public API (with a
Perl_prefix) or internal (often with anS_prefix) and declared asstatic. A few symbols inutil.chave been overlooked: They are clearly intended to be internal (no prefix, only used within this file), but not declaredstatic, so they were visible to the linker. These patches add the missingstatickeyword to those symbols.