Skip to content

gh-118124: fix assert related C++ checks on Solaris/Illumos#121974

Merged
vstinner merged 2 commits intopython:mainfrom
kulikjak:fix-static_assert-detection
Jul 21, 2024
Merged

gh-118124: fix assert related C++ checks on Solaris/Illumos#121974
vstinner merged 2 commits intopython:mainfrom
kulikjak:fix-static_assert-detection

Conversation

@kulikjak
Copy link
Copy Markdown
Contributor

@kulikjak kulikjak commented Jul 18, 2024 •
edited by bedevere-app bot
Loading

__STDC_VERSION__ can be defined for C++ builds on some platforms (in my case, Solaris/Illumos).

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=57025

@bedevere-app bedevere-app bot mentioned this pull request Jul 18, 2024
Closed
namniav
namniav suggested changes Jul 18, 2024
View reviewed changes
Include/pymacro.h Outdated
// not defined), and only for C11-17.
#if !defined(static_assert) && (defined(__GNUC__) || defined(__clang__)) \
&& defined(__STDC_VERSION__) && __STDC_VERSION__ >= 201112L \
&& !defined(__cplusplus) && __STDC_VERSION__ >= 201112L \
Copy link
Copy Markdown

@namniav namniav Jul 18, 2024

Choose a reason for hiding this comment

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

I'd keep defined(__STDC_VERSION__) and add && !defined(__cplusplus) to the end. I believe it was there for some reason, e.g. GCC and Clang may warn undefined identifiers(-Wundef).

Copy link
Copy Markdown
Contributor Author

@kulikjak kulikjak Jul 21, 2024

Choose a reason for hiding this comment

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

You are right, better be safe here. I kept the defined(__STDC_VERSION__) in the check.

@picnixz
Copy link
Copy Markdown
Member

picnixz commented Jul 21, 2024

cc @vstinner as the author of the original fix

@vstinner vstinner merged commit e88bd96 into python:main Jul 21, 2024
@vstinner vstinner added the needs backport to 3.13 bugs and security fixes label Jul 21, 2024
@miss-islington-app
Copy link
Copy Markdown

miss-islington-app bot commented Jul 21, 2024

Thanks @kulikjak for the PR, and @vstinner for merging it 🌮🎉.. I'm working now to backport this PR to: 3.13.
🐍🍒⛏🤖

@miss-islington-app
Copy link
Copy Markdown

miss-islington-app bot commented Jul 21, 2024

Sorry, @kulikjak and @vstinner, I could not cleanly backport this to 3.13 due to a conflict.
Please backport using cherry_picker on command line.

cherry_picker e88bd96d0d6cf8218c4fca37e1d20399ae676a04 3.13

@vstinner
Copy link
Copy Markdown
Member

vstinner commented Jul 21, 2024

Do 3.12 and 3.13 branches need backport? I cannot check right now.

@kulikjak
Copy link
Copy Markdown
Contributor Author

kulikjak commented Jul 21, 2024

So, the second #ifdef (the Py_BUILD_ASSERT_EXPR related one you added a few months ago) is in 3.13 as well, but doesn't backport cleanly because there was an unrelated push into main since.

The first #ifdef is all the way in 3.11, though I don't think that branch accepts bug fixes anymore (3.12 does).

I can prepare backport PRs.

@vstinner
Copy link
Copy Markdown
Member

vstinner commented Jul 21, 2024

If 3.12 and 3.13, backports would be welcomed, sure.

kulikjak added a commit to kulikjak/cpython that referenced this pull request Jul 22, 2024
@kulikjak
[3.13] pythongh-118124: fix assert related C++ checks on Solaris/Illu…
6ff5919 
…mos (pythonGH-121974)

Fix check for static_assert() for C++ on some platforms..
(cherry picked from commit e88bd96)

Co-authored-by: Jakub Kulík <Kulikjak@gmail.com>
@bedevere-app
Copy link
Copy Markdown

bedevere-app bot commented Jul 22, 2024

GH-122108 is a backport of this pull request to the 3.13 branch.

@bedevere-app bedevere-app bot removed the needs backport to 3.13 bugs and security fixes label Jul 22, 2024
@bedevere-app
Copy link
Copy Markdown

bedevere-app bot commented Jul 22, 2024

GH-122109 is a backport of this pull request to the 3.12 branch.

vstinner pushed a commit that referenced this pull request Jul 22, 2024
@kulikjak
[3.12] gh-118124: fix assert related C++ checks on Solaris/Illumos (G…
6586b17 
…H-121974) (#122109)

Fix check for static_assert() for C++ on some platforms..
(cherry picked from commit e88bd96)
vstinner pushed a commit that referenced this pull request Jul 22, 2024
@kulikjak
[3.13] gh-118124: fix assert related C++ checks on Solaris/Illumos (G…
82db815 
…H-121974) (#122108)

Fix check for static_assert() for C++ on some platforms..
(cherry picked from commit e88bd96)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@namniav namniav namniav approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@vstinner vstinner

Labels

skip news

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@kulikjak @picnixz @vstinner @namniav