Skip to content

Conversation

@rwgk
Copy link
Collaborator

@rwgk rwgk commented Nov 29, 2025

Description

Closes contourpy/contourpy#512

Validations:

Suggested changelog entry:

  • A workaround for a GCC -Warray-bounds false positive in argument_vector was added.

@rwgk
Copy link
Collaborator Author

rwgk commented Nov 29, 2025

@swolchok does this look good to you? (Could you please approve?)

@swolchok
Copy link
Contributor

@swolchok does this look good to you? (Could you please approve?)

Don’t you need to gate the push and the pop with the same conditions?

@rwgk
Copy link
Collaborator Author

rwgk commented Nov 30, 2025

Don’t you need to gate the push and the pop with the same conditions?

It's critical that the push/pop pair are balanced, but they don't have to be guarded. There are existing similar cases, for example here:

  • PYBIND11_WARNING_PUSH
    #ifdef PYBIND11_DETECTED_CLANG_WITH_MISLEADING_CALL_STD_MOVE_EXPLICITLY_WARNING
    PYBIND11_WARNING_DISABLE_CLANG("-Wreturn-std-move")
    #endif
    return result;
    PYBIND11_WARNING_POP

The PYBIND11_WARNING_* macro system is designed so that we don't have to repeat complex #ifs. An extra PYBIND11_WARNING_PUSH/POP when they don't end up disabling anything is basically a no-op at compile time. It's just one more diagnostic push/pop pragma for the compiler to parse, which I believe is immeasurable compared to the cost of processing the surrounding templates.

Copy link
Contributor

@swolchok swolchok left a comment

Choose a reason for hiding this comment

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

Seems fine then

@rwgk
Copy link
Collaborator Author

rwgk commented Nov 30, 2025

Thanks @swolchok!

@rwgk rwgk merged commit 55e4bb9 into pybind:master Nov 30, 2025
147 of 148 checks passed
@github-actions github-actions bot added the needs changelog Possibly needs a changelog entry label Nov 30, 2025
@rwgk rwgk deleted the fix_contourpy_issues_512 branch November 30, 2025 18:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs changelog Possibly needs a changelog entry

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Build failure with master branch of pybind11

2 participants