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 — bpo-20291: Fix MSVC warnings in getargs.c by Fidget-Spinner · Pull Request #27211 · python/cpython

Conversation

@Fidget-Spinner

@Fidget-Spinner Fidget-Spinner commented Jul 17, 2021 •
edited by bedevere-bot
Loading

Uh oh!

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

Copy link
Copy Markdown
Member

https://bugs.python.org/issue20291

Sorry, something went wrong.

Uh oh!

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

All reactions
@Fidget-Spinner
Fix MSVC warnings in getargs.c
e472b85
@bedevere-bot bedevere-bot added the awaiting review label Jul 17, 2021
@the-knights-who-say-ni the-knights-who-say-ni added the CLA signed label Jul 17, 2021
@Fidget-Spinner Fidget-Spinner added the skip news label Jul 17, 2021
@Fidget-Spinner
Fidget-Spinner requested a review from isidentical July 17, 2021 09:09
@Fidget-Spinner

Fidget-Spinner commented Jul 17, 2021 •
edited
Loading

Uh oh!

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

Copy link
Copy Markdown
Member Author

Currently:

Check warning on line 2555 in Python/getargs.c

GitHub Actions / Windows (x64)
Python/getargs.c#L2555
'=': conversion from 'Py_ssize_t' to 'int', possible loss of data [D:\a\cpython\cpython\PCbuild\pythoncore.vcxproj]

posonly is already int, so I think varargssize can be int without overflow.

All reactions

Sorry, something went wrong.

Uh oh!

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

isidentical
isidentical reviewed Jul 17, 2021
View reviewed changes
Comment thread Python/getargs.c Resolved

Uh oh!

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

@isidentical

isidentical commented Jul 17, 2021

Copy link
Copy Markdown
Member

posonly is already int, so I think varargssize can be int without overflow.

Yep. As an extra guard we can just use Py_SAFE_DOWNCAST.

All reactions

Sorry, something went wrong.

Uh oh!

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

@Fidget-Spinner @isidentical
apply suggestions
6c1b377
Co-Authored-By: Batuhan Taskaya <[email protected]>
isidentical
isidentical approved these changes Jul 17, 2021
View reviewed changes

@isidentical isidentical left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

LGTM

Sorry, something went wrong.

Uh oh!

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

All reactions
@bedevere-bot bedevere-bot added awaiting merge and removed awaiting review labels Jul 17, 2021
@isidentical

isidentical commented Jul 17, 2021

Copy link
Copy Markdown
Member

test_asyncio failure, restarting the CI.

All reactions

Sorry, something went wrong.

Uh oh!

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

@isidentical
isidentical merged commit f88e138 into python:main Jul 17, 2021
@bedevere-bot bedevere-bot removed the awaiting merge label Jul 17, 2021
sthagen added a commit to sthagen/python-cpython that referenced this pull request Jul 17, 2021
@sthagen
Merge pull request #533 from python/main
0f61e2b
bpo-20291: Fix MSVC warnings in getargs.c (pythonGH-27211)
@Fidget-Spinner
Fidget-Spinner deleted the fix_msvc_warnings branch July 17, 2021 16:21
shihai1991 added a commit to shihai1991/cpython that referenced this pull request Jul 20, 2021
@shihai1991
Merge remote-tracking branch 'origin/main' into bpo_42035
56e8889
* origin/main: (1146 commits)
  bpo-42064: Finalise establishing sqlite3 global state (pythonGH-27155)
  bpo-44678: Separate error message for discontinuous padding in binascii.a2b_base64 strict mode (pythonGH-27249)
  correct spelling (pythonGH-27076)
  bpo-44524: Add missed __name__ and __qualname__ to typing module objects (python#27237)
  bpo-27513: email.utils.getaddresses() now handles Header objects (python#13797)
  Clean up comma usage in Doc/library/functions.rst (python#27083)
  bpo-42238: Fix small rst issue in NEWS.d/. (python#27238)
  bpo-41972: Tweak fastsearch.h string search algorithms (pythonGH-27091)
  bpo-44340: Add support for building with clang full/thin lto (pythonGH-27231)
  bpo-44661: Update property_descr_set to use vectorcall if possible. (pythonGH-27206)
  bpo-44645: Check for interrupts on any potentially backwards edge (pythonGH-27216)
  bpo-41546: make pprint (like print) not write to stdout when it is None (pythonGH-26810)
  bpo-44554: refactor pdb targets (and internal tweaks) (pythonGH-26992)
  bpo-43086: Add handling for out-of-spec data in a2b_base64 (pythonGH-24402)
  bpo-44561: Update hyperlinks in Doc/distributing/index.rst (python#27032)
  bpo-42355: symtable.get_namespace() now checks whether there are multiple or any namespaces found (pythonGH-23278)
  bpo-44654: Do not export the union type related symbols (pythonGH-27223)
  bpo-44633: Fix parameter substitution of the union type with wrong types. (pythonGH-27218)
  bpo-44654: Refactor and clean up the union type implementation (pythonGH-27196)
  bpo-20291: Fix MSVC warnings in getargs.c (pythonGH-27211)
  ...
@vstinner

vstinner commented Sep 7, 2021

Copy link
Copy Markdown
Member

I dislike Py_SAFE_DOWNCAST(). Usually, I prefer to use a wider type (use Py_ssize_t for "i"), rather than using downcasting.

All reactions

Sorry, something went wrong.

Uh oh!

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

@vstinner

vstinner commented Sep 7, 2021

Copy link
Copy Markdown
Member

Well, in practice, a function should have less than 2^31 arguments :-)

All reactions

Sorry, something went wrong.

Uh oh!

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

@larryhastings larryhastings mentioned this pull request Apr 10, 2022
Argument Clinic should understand *args and **kwargs parameters #64490
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@isidentical isidentical isidentical approved these changes

Uh oh!

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

Assignees

No one assigned

Labels

skip news

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

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

5 participants

@Fidget-Spinner @isidentical @vstinner @the-knights-who-say-ni @bedevere-bot
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 01402520 — 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.