-
Notifications
You must be signed in to change notification settings - Fork 595
regcomp(_internal)?.c: Avoid some casts in prints #23728
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.
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
base: blead
Are you sure you want to change the base?
Conversation
regcomp.c
Outdated
(IV)val, | ||
(IV)(val - scan) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You left the (IV)
casts in; do you need to remove them for %zd
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I should have removed the casts; now done
We have modifiers like %z and %t to portably print items of the given type. This converts to use those in these files, removing casts that were previously used.
33edab8
to
3ca8d66
Compare
Perl_re_printf( aTHX_ "~ tying lastbr %s (%" IVdf ") to ender %s (%" IVdf ") offset %" IVdf "\n", | ||
Perl_re_printf( aTHX_ "~ tying lastbr %s (%" IVdf ") to ender %s (%" IVdf ") offset %td\n", | ||
SvPV_nolen_const(RExC_mysv1), | ||
(IV)lastbr, | ||
SvPV_nolen_const(RExC_mysv2), | ||
(IV)ender, | ||
(IV)(ender - lastbr) | ||
(ender - lastbr) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure if you're distinguishing ptrdiff_t
and ssize_t
here, or if you're using the assumption that regnode_offset
is SSize_t
(which it is, but I'm not sure you intend that to be visible.)
ender
and lastbr
are regnode_offset
s which are SSize_t
but you've retained the casts and IVdf
for each of those.
I'd expect subtracting lastbr
from ender
to also be a SSize_t
(%zd) but you've used %td
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Similar cases below, let me know if you want me to point each of them out.
* lib/unicore/Name.pm. 'code_point' in the name is expressed in hex. */ | ||
for (i = 0; i <= av_top_index((AV *) algorithmic_names); i++) { | ||
IV j; | ||
SSize_t j; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I understand the code low
and high
represent a range of code points, and so hence j
is also a code point.
What I'm not sure about is what controls the Perl extended valid range of code points, if it's limited to non-negative IV
values this change is incorrect, since IV/UV can be larger than SSize_t
.
It might be that the range here is always within the base unicode range, but I'm not sure we limit custom properties to that (or if those apply here.)
We have modifiers like %z and %t to portably print items of the given type. This converts to use those in these files, removing casts that were previously used.