Skip to content

Fix integer promotion causing a warning in MSVC #111

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

Merged
merged 1 commit into from
Apr 24, 2022

Conversation

ltrzesniewski
Copy link
Contributor

@ltrzesniewski ltrzesniewski commented Apr 23, 2022

This continues the discussion from #105.

When compiled on x86, MSVC generates the following warning (which becomes an error when the Treat Warnings As Errors option is used):

pcre2_jit_compile.c(5889,40): warning C4018: '>=': signed/unsigned mismatch

on the following line:

if (b_pri > 2 && a_pri + b_pri >= max_pri)

This warning seems wrong at first, as a_pri and b_pri are of type PCRE2_UCHAR, while max_pri is of type sljit_u32, so both operands are supposed to be unsigned.

Turns out, implicit conversion rules in C are... complicated. The way I understand them, the a_pri + b_pri expression is promoted to type int, which causes the signed/unsigned mismatch on >=.

Here are the relevant quotes from the Integer promotions section:

Integer promotion is the implicit conversion of a value of any integer type with rank less or equal to rank of int or of a bit field of type _Bool, int, signed int, unsigned int, to the value of type int or unsigned int

If int can represent the entire range of values of the original type (or the range of values of the original bit field), the value is converted to type int. Otherwise the value is converted to unsigned int.

Note: integer promotions are applied only:
[...]
to the operand of the unary arithmetic operators + and -
[...]

When applied to the a_pri + b_pri expression:

  • In the 8-bit and 16-bit versions of the library, the rank of PCRE2_UCHAR is less than the rank of int (see the definition of rank in the linked article), and equal in the 32-bit version, therefore the integer promotion rules apply.

  • In the 8-bit and 16-bit versions, int can represent the entire range of values of PCRE2_UCHAR, therefore the expression type gets promoted to type int and the warning is raised.

  • In the 32-bit version, int cannot represent the entire range of values of PCRE2_UCHAR, therefore unsigned int is used and no warning is raised.

I'm not sure why the warning is not raised on x64 though. Since Windows uses the LLP64 data model, I believe it should have raised the same warning as on x86, but I didn't dig deeper.

This PR casts a_pri to sljit_u32, which forces the expression to an unsigned type. Note that this is the type of max_pri, to which the addition is assigned later on.

Copy link
Collaborator

@zherczeg zherczeg left a comment

Choose a reason for hiding this comment

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

LGTM

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