Skip to content

fix: allow -Wpedantic in C++20 mode - #5322

Merged
henryiii merged 5 commits into
pybind:masterfrom
henryiii:henryiii/fix/pedantic
Aug 22, 2024
Merged

fix: allow -Wpedantic in C++20 mode#5322
henryiii merged 5 commits into
pybind:masterfrom
henryiii:henryiii/fix/pedantic

Conversation

@henryiii

@henryiiihenryiii commented Aug 20, 2024

Copy link
Copy Markdown
Collaborator

Description

I think we can support -Wpedandic in C++20 mode. Before that the variadic macros cause warnings that are pretty much impossible to fix from a macro AFAICT. Fix#5310.

Suggested changelog entry:

* Support ``-Wpedantic`` in C++20 mode

@henryiii
henryiiiforce-pushed the henryiii/fix/pedantic branch 3 times, most recently from 37cd500 to 10c5398CompareAugust 20, 2024 22:08
@henryiii
henryiii marked this pull request as draft August 21, 2024 15:33
@henryiii
henryiiiforce-pushed the henryiii/fix/pedantic branch 2 times, most recently from 6605ad4 to bdf929cCompareAugust 21, 2024 16:42
Signed-off-by: Henry Schreiner <henryschreineriii@gmail.com>
@henryiii
henryiiiforce-pushed the henryiii/fix/pedantic branch from bdf929c to 1a6fca3CompareAugust 21, 2024 16:50
Signed-off-by: Henry Schreiner <henryschreineriii@gmail.com>
@henryiii
henryiiiforce-pushed the henryiii/fix/pedantic branch from 6e02b58 to b0e2ac3CompareAugust 21, 2024 17:22
Signed-off-by: Henry Schreiner <henryschreineriii@gmail.com>
@henryiiihenryiii changed the title fix: allow -Wpedantic againfix: allow -Wpedantic in C++20 modeAug 21, 2024
@henryiii
henryiiiforce-pushed the henryiii/fix/pedantic branch from 6a8463d to 641f9baCompareAugust 21, 2024 17:49
Signed-off-by: Henry Schreiner <henryschreineriii@gmail.com>
@henryiii
henryiiiforce-pushed the henryiii/fix/pedantic branch from 641f9ba to b3d5042CompareAugust 21, 2024 17:54
Comment threadtests/local_bindings.h Outdated

@rwgkrwgk left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks for the cleanup!
I think it's a great compromise, to turn on the option only for C++20. That'll will catch pretty much everything already.


\endrst */
PYBIND11_WARNING_PUSH
PYBIND11_WARNING_DISABLE_CLANG("-Wgnu-zero-variadic-macro-arguments")

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Just double-checking: is this needed, even with -std=c++20?

@henryiiihenryiiiAug 22, 2024

Copy link
Copy Markdown
CollaboratorAuthor

Choose a reason for hiding this comment

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

Yeah, for some reason Clang produces a warning here. GCC is fine, but Clang complains. It produces a warning saying this is a C++20 feature on newer compilers if set less than C++20, so no idea why the old warning triggers in C++20 mode. I think it's a bug.

@henryiii
henryiii merged commit 9966ad4 into pybind:masterAug 22, 2024
@henryiii
henryiii deleted the henryiii/fix/pedantic branch August 22, 2024 04:27
@github-actionsgithub-actionsBot added the needs changelog Possibly needs a changelog entry label Aug 22, 2024
henryiii added a commit that referenced this pull request Aug 22, 2024
* fix: allow -Wpedantic again
Signed-off-by: Henry Schreiner <henryschreineriii@gmail.com>
* tests: ignore pedantic warning for PYBIND11_DECLARE_HOLDER_TYPE
Signed-off-by: Henry Schreiner <henryschreineriii@gmail.com>
* tests: try just turning off pedantic for one file
Signed-off-by: Henry Schreiner <henryschreineriii@gmail.com>
* tests: only run pedantic in C++20 mode
Signed-off-by: Henry Schreiner <henryschreineriii@gmail.com>
* Update tests/local_bindings.h
---------
Signed-off-by: Henry Schreiner <henryschreineriii@gmail.com>
@henryiiihenryiii removed the needs changelog Possibly needs a changelog entry label Sep 13, 2024
Sign up for freeto 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.

[BUG]: token pasting of ',' and __VA_ARGS__ is a GNU extension / pedantic warnings

2 participants

@henryiii@rwgk