Skip to content

fix: avoid header pollution from Mersenne Twister helper macros #70

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
dkkoma wants to merge 1 commit into from
Closed

fix: avoid header pollution from Mersenne Twister helper macros #70

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions ext/colopl_bc_php70.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,15 @@ PHP_FUNCTION(Colopl_ColoplBc_Php70_getrandmax)

/* mt_rand.c */

#define N COLOPL_BC_MT_N /* length of state vector */
#define M (397) /* a period parameter */
#define hiBit(u) ((u) & 0x80000000U) /* mask all but highest bit of u */
#define loBit(u) ((u) & 0x00000001U) /* mask all but lowest bit of u */
#define loBits(u) ((u) & 0x7FFFFFFFU) /* mask the highest bit of u */
#define mixBits(u, v) (hiBit(u)|loBits(v)) /* move hi bit of u to hi bit of v */

#define twist(m,u,v) (m ^ (mixBits(u,v)>>1) ^ ((uint32_t)(-(uint32_t)(loBit(u))) & 0x9908b0dfU))

Comment on lines +145 to +153
Copy link

Copilot AI Apr 20, 2026

Choose a reason for hiding this comment

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

These helper macros (N, M, hiBit/loBit/loBits/mixBits, twist) are now TU-local, but they remain defined for the rest of this .c file. To avoid accidental macro expansion/collisions in the later sections of the file (array.c/string.c) and to keep the scope tight, undefine them after the MT implementation (e.g., right before the /* array.c */ section).

Suggested change
#define N COLOPL_BC_MT_N /* length of state vector */
#define M (397) /* a period parameter */
#define hiBit(u) ((u) & 0x80000000U) /* mask all but highest bit of u */
#define loBit(u) ((u) & 0x00000001U) /* mask all but lowest bit of u */
#define loBits(u) ((u) & 0x7FFFFFFFU) /* mask the highest bit of u */
#define mixBits(u, v) (hiBit(u)|loBits(v)) /* move hi bit of u to hi bit of v */
#define twist(m,u,v) (m ^ (mixBits(u,v)>>1) ^ ((uint32_t)(-(uint32_t)(loBit(u))) & 0x9908b0dfU))
enum {
N = COLOPL_BC_MT_N, /* length of state vector */
M = 397 /* a period parameter */
};
static inline uint32_t hiBit(uint32_t u) /* mask all but highest bit of u */
{
return u & 0x80000000U;
}
static inline uint32_t loBit(uint32_t u) /* mask all but lowest bit of u */
{
return u & 0x00000001U;
}
static inline uint32_t loBits(uint32_t u) /* mask the highest bit of u */
{
return u & 0x7FFFFFFFU;
}
static inline uint32_t mixBits(uint32_t u, uint32_t v) /* move hi bit of u to hi bit of v */
{
return hiBit(u) | loBits(v);
}
static inline uint32_t twist(uint32_t m, uint32_t u, uint32_t v)
{
return m ^ (mixBits(u, v) >> 1) ^ ((uint32_t)(-(uint32_t)(loBit(u))) & 0x9908b0dfU);
}

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Collaborator

@zeriyoshi zeriyoshi Apr 20, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

@dkkoma
Thanks! However, the twist macro is used here, so it cannot be removed. It gets called when MT19937 needs to reload, but since this only happens under very limited conditions, it appears to be slipping through the tests.

https://github.com/colopl/php-colopl_bc/blob/main/ext/colopl_bc_php70.c#L165

Since there's no real benefit to using a macro in the current C implementation, could you redefine it as an inline function with a different name, such as static inline colopl_bc_twist() ?

static inline void mt_initialize(uint32_t seed, uint32_t *state)
{
register uint32_t *s = state;
Expand Down
2 changes: 1 addition & 1 deletion ext/php_colopl_bc.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ ZEND_BEGIN_MODULE_GLOBALS(colopl_bc)
bool rand_is_seeded;
bool mt_rand_is_seeded;
uint32_t rand_seed;
uint32_t mt_state[N+1];
uint32_t mt_state[COLOPL_BC_MT_N+1];
uint32_t *mt_next;
int mt_left;
int gnurandom_r[344];
Expand Down
9 changes: 1 addition & 8 deletions ext/php_colopl_bc_php70.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,14 +113,7 @@ PHPAPI zend_long php_colopl_bc_rand(void);

#define PHP_COLOPL_BC_MT_RAND_MAX ((zend_long) (0x7FFFFFFF)) /* (1<<31) - 1 */

#define N (624) /* length of state vector */
#define M (397) /* a period parameter */
#define hiBit(u) ((u) & 0x80000000U) /* mask all but highest bit of u */
#define loBit(u) ((u) & 0x00000001U) /* mask all but lowest bit of u */
#define loBits(u) ((u) & 0x7FFFFFFFU) /* mask the highest bit of u */
#define mixBits(u, v) (hiBit(u)|loBits(v)) /* move hi bit of u to hi bit of v */

#define twist(m,u,v) (m ^ (mixBits(u,v)>>1) ^ ((uint32_t)(-(uint32_t)(loBit(u))) & 0x9908b0dfU))
#define COLOPL_BC_MT_N (624)

PHPAPI void php_colopl_bc_mt_srand(uint32_t seed);
PHPAPI uint32_t php_colopl_bc_mt_rand(void);
Expand Down
Loading