GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — GH-93207: Remove configure check for stdarg.h (HAVE_STDARG_PROTOTYPES) by kumaraditya303 · Pull Request #93215 · python/cpython

Conversation

@kumaraditya303

@kumaraditya303 kumaraditya303 commented May 25, 2022 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

Closes #93207

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
kumaraditya303 added 2 commits May 25, 2022 07:06
@kumaraditya303
remove check for HAVE_STDARG_PROTOTYPES
4616854
@kumaraditya303
autoconf
c9a6cc4
@kumaraditya303
kumaraditya303 requested review from 1st1, asvetlov, iritkatriel, lysnikolaou, markshannon and pablogsal as code owners May 25, 2022 10:48
@bedevere-bot bedevere-bot added the awaiting review label May 25, 2022
@kumaraditya303 kumaraditya303 mentioned this pull request May 25, 2022
Remove check for stdarg.h (HAVE_STDARG_PROTOTYPES) in configure.ac #93207
Closed
@erlend-aasland

erlend-aasland commented May 25, 2022 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

AC changes look good, but I would try to investigate further which compilers/build environments this will affect (if any); buildbots do not cover everything.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@kumaraditya303

kumaraditya303 commented May 25, 2022

Copy link
Copy Markdown
Contributor Author

AC changes look good, but I would try to investigate further which compilers/build environments this will affect (if any); buildbots do not cover everything.

It is unlikely as there is already code which uses variadic args and does not checks for HAVE_STDARG_PROTOTYPES and is working.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@kumaraditya303 kumaraditya303 self-assigned this May 25, 2022
@kumaraditya303 kumaraditya303 added the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label May 25, 2022
@bedevere-bot

bedevere-bot commented May 25, 2022

Copy link
Copy Markdown

🤖 New build scheduled with the buildbot fleet by @kumaraditya303 for commit c9a6cc4 🤖

If you want to schedule another build, you need to add the ":hammer: test-with-buildbots" label again.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@bedevere-bot bedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label May 25, 2022
@blurb-it
📜🤖 Added by blurb_it.
e21b810
@kumaraditya303

kumaraditya303 commented May 25, 2022

Copy link
Copy Markdown
Contributor Author

So far all buildbots passed, I added a news item.

cc @vstinner

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@kumaraditya303
kumaraditya303 requested a review from vstinner May 25, 2022 13:57
@kumaraditya303

kumaraditya303 commented May 25, 2022 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor Author

@erlend-aasland: va_start macro requires two arguments1 and if you try to replace va_start(vargs, format); with va_start(vargs); then it won't even compile. I tried it and got this compilation error proving that va_start with one argument is wrong. Hence this PR is correct and fixes the issue.

/workspaces/cpython/Modules/_asynciomodule.c:2646:19: error: macro "va_start" requires 2 arguments, but only 1 given
 2646 |     va_start(vargs);
      |                   ^

Footnotes

  1. https://www.cplusplus.com/reference/cstdarg/va_start/ ↩

erlend-aasland reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

vstinner
vstinner reviewed May 26, 2022
View reviewed changes
Comment thread Misc/NEWS.d/next/Build/2022-05-25-13-56-00.gh-issue-93207.B9Rubf.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.

kumaraditya303 added 2 commits May 27, 2022 06:16
@kumaraditya303
Merge branch 'main' of https://github.com/python/cpython into pythong…
f0ef047
…h-93207
@kumaraditya303
fix news
cb0d6d6
@kumaraditya303
kumaraditya303 requested a review from vstinner May 27, 2022 07:20
erlend-aasland
erlend-aasland approved these changes May 27, 2022
View reviewed changes
@bedevere-bot bedevere-bot removed the awaiting review label May 27, 2022
@vstinner

vstinner commented May 27, 2022 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

Many files already call va_start() with 2 arguments: Objects/call.c, PC/launcher.c and PC/launcher2.c, Parser/pegen_errors.c, etc. (Without testing if the HAVE_STDARG_PROTOTYPES macro is defined.)

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@vstinner

vstinner commented May 27, 2022

Copy link
Copy Markdown
Member

Please remove also this line from PC/pyconfig.h:

/* Define if you have the <stdarg.h> prototypes.  */
#define HAVE_STDARG_PROTOTYPES
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@kumaraditya303
fix windows
07c89ee
@kumaraditya303
kumaraditya303 requested a review from a team as a code owner May 27, 2022 10:28
@kumaraditya303

kumaraditya303 commented May 27, 2022

Copy link
Copy Markdown
Contributor Author

Many files already call va_start() with 2 arguments: Objects/call.c, PC/launcher.c and PC/launcher2.c, Parser/pegen_errors.c, etc. (Without testing if the HAVE_STDARG_PROTOTYPES macro is defined.)

Done

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@vstinner
vstinner merged commit cb04a09 into python:main May 27, 2022
@vstinner

vstinner commented May 27, 2022

Copy link
Copy Markdown
Member

Merged, thanks.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@kumaraditya303
kumaraditya303 deleted the gh-93207 branch May 27, 2022 11:31
@tacaswell

tacaswell commented Jun 2, 2022

Copy link
Copy Markdown
Contributor

This broke cython which appears to have been relying on HAVE_STDARG_PROTOTYPES to be defined by CPython. I am going to open a PR an the cython side to always go with the 2 argument version.

erlend-aasland reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@tacaswell tacaswell mentioned this pull request Jun 2, 2022
MNT: always require va_start to have two arguments cython/cython#4820
Merged
da-woods pushed a commit to cython/cython that referenced this pull request Jun 7, 2022
@tacaswell
MNT: always require va_start to have two arguments (#4820)
4613ec0
* MNT: always require va_start to have two arguments

python/cpython#93215 chance CPython to always use the
2-input version of va_start and dropped defining HAVE_STDARG_PROTOTYPES.  This
resulted in the 1-argument version being used when compiling cython source
which fails

This makes cython also always use the 2-argument version.

* Remove blank line

* FIX: version gate 2-argument va_start checking to py311
da-woods pushed a commit to cython/cython that referenced this pull request Jun 7, 2022
@tacaswell @da-woods
MNT: always require va_start to have two arguments (#4820)
ad2d1f7
* MNT: always require va_start to have two arguments

python/cpython#93215 chance CPython to always use the
2-input version of va_start and dropped defining HAVE_STDARG_PROTOTYPES.  This
resulted in the 1-argument version being used when compiling cython source
which fails

This makes cython also always use the 2-argument version.

* Remove blank line

* FIX: version gate 2-argument va_start checking to py311
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@erlend-aasland erlend-aasland erlend-aasland approved these changes
@pablogsal pablogsal Awaiting requested review from pablogsal pablogsal is a code owner
@lysnikolaou lysnikolaou Awaiting requested review from lysnikolaou lysnikolaou is a code owner
@iritkatriel iritkatriel Awaiting requested review from iritkatriel iritkatriel is a code owner
@markshannon markshannon Awaiting requested review from markshannon markshannon is a code owner
@1st1 1st1 Awaiting requested review from 1st1 1st1 is a code owner
@asvetlov asvetlov Awaiting requested review from asvetlov asvetlov is a code owner
@vstinner vstinner Awaiting requested review from vstinner

Uh oh!

There was an error while loading. Please reload this page.

Assignees

@kumaraditya303 kumaraditya303

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Remove check for stdarg.h (HAVE_STDARG_PROTOTYPES) in configure.ac

Uh oh!

There was an error while loading. Please reload this page.

5 participants

@kumaraditya303 @erlend-aasland @bedevere-bot @vstinner @tacaswell
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.