Skip to content

Conversation

@g-abilio
Copy link
Contributor

@g-abilio g-abilio commented Dec 18, 2025

Type of change

  • Bug fix

Description

When compiling the project using MSVC in Qt Creator, the following warnings are displayed:

 warning C4267: 'argument': conversion from 'size_t' to 'const QtNodes::PortIndex', possible loss of data
 cl : Command line warning D9025 : overriding '/W3' with '/W4'

This PR aims to solve these warnings.

Testing

  • Qt version tested:
  • Existing tests still pass
  • Added tests for new functionality (if applicable)

@paceholder
Copy link
Owner

Hi, thanks for this fix.

Do you know where the conflict /W3 vs /W4 comes from?

@Llcoolsouder
Copy link
Collaborator

Llcoolsouder commented Jan 5, 2026

Hi, thanks for this fix.

Do you know where the conflict /W3 vs /W4 comes from?

Looks like we're adding it in explicitly here:

$<$<CXX_COMPILER_ID:MSVC>:/W4 /wd4127 /EHsc /utf-8>

Might be better to remove it here instead.
Probably want to make sure we're not removing warnings entirely though—which at a glance, seems to be what this PR does.

@g-abilio
Copy link
Contributor Author

g-abilio commented Jan 6, 2026

Hi, thanks for this fix.
Do you know where the conflict /W3 vs /W4 comes from?

Looks like we're adding it in explicitly here:

$<$<CXX_COMPILER_ID:MSVC>:/W4 /wd4127 /EHsc /utf-8>

Might be better to remove it here instead. Probably want to make sure we're not removing warnings entirely though—which at a glance, seems to be what this PR does.

Thanks for the suggestion! It really does seem like a better option for solving the warnings.

Regarding removing the warnings, could you explain why solving all the override and casting warnings that were mentioned wouldn't be a good idea?

@Llcoolsouder
Copy link
Collaborator

Regarding removing the warnings, could you explain why solving all the override and casting warnings that were mentioned wouldn't be a good idea?

I may have left that a bit unclear.
To clarify, it's definitely a good idea to fix warnings. I only meant that we should double check that this line:

string(REGEX REPLACE "/W[0-4]" "" CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS}")

isn't just turning off/silencing all warnings.

Looking at the code a bit closer now, I see that your regex deleting the /W flags comes just before a line that adds /W4, so I think you're good to go.


To close the loop on where this conflict even came from, the Cmake docs seem to indicate that /W3 is a part of the set of default flags used in MSVC in CMake <3.15

@Llcoolsouder Llcoolsouder self-assigned this Jan 6, 2026
@Llcoolsouder Llcoolsouder merged commit 3799f43 into paceholder:master Jan 6, 2026
3 checks passed
@g-abilio
Copy link
Contributor Author

g-abilio commented Jan 6, 2026

Regarding removing the warnings, could you explain why solving all the override and casting warnings that were mentioned wouldn't be a good idea?

I may have left that a bit unclear. To clarify, it's definitely a good idea to fix warnings. I only meant that we should double check that this line:

string(REGEX REPLACE "/W[0-4]" "" CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS}")

isn't just turning off/silencing all warnings.

Looking at the code a bit closer now, I see that your regex deleting the /W flags comes just before a line that adds /W4, so I think you're good to go.

To close the loop on where this conflict even came from, the Cmake docs seem to indicate that /W3 is a part of the set of default flags used in MSVC in CMake <3.15

Great! Thanks for the help and the clarification @Llcoolsouder!

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