Skip to content

Fix warning about signed/unsigned mismatch#649

Merged
zherczeg merged 1 commit intoPCRE2Project:masterfrom
ltrzesniewski:fix-warn
Dec 27, 2024
Merged

Fix warning about signed/unsigned mismatch#649
zherczeg merged 1 commit intoPCRE2Project:masterfrom
ltrzesniewski:fix-warn

Conversation

@ltrzesniewski
Copy link
Contributor

Congrats on the RC1 🙂

I've got the following warning when trying to compile it with MSVC though, which is a problem when you turn warning as errors on:

Warning C4018 : '>': signed/unsigned mismatch

(I supposed the PR should target the master branch)

@NWilson
Copy link
Member

NWilson commented Dec 27, 2024

Thank you very much! Our windows CI builds don't have all the warnings enabled, unfortunately, so that's how this got past testing. There's an existing issue in my personal task list to tighten up on them.

I'll let Zoltan review and merge, but the fix is fine to me. Adding "u" after the number constants would also work, if you wanted to avoid committing to a particular bit width.

Merging to master is correct.

Master is the new playground, open to all changes big and small, and I'll do the admin to cherry pick any fixes like this onto the final release branch when the time comes. (@zherczeg)

@ltrzesniewski
Copy link
Contributor Author

ltrzesniewski commented Dec 27, 2024

Oh, I should have mentioned I already had an issue like this in the past, see #111.

I used a cast because I did so in the previous PR, there already were a number of those in the file, and because max is a sljit_u32 (so there's no bit width issue).

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

@zherczeg zherczeg merged commit 35eafc9 into PCRE2Project:master Dec 27, 2024
31 checks passed
@ltrzesniewski ltrzesniewski deleted the fix-warn branch December 27, 2024 18:31
@NWilson NWilson mentioned this pull request Feb 4, 2025
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