Skip to content

Fix segfault when a single arg is passed to setdefault#1160

Merged
bdraco merged 3 commits intomasterfrom
single_arg_setdefault
May 16, 2025
Merged

Fix segfault when a single arg is passed to setdefault#1160
bdraco merged 3 commits intomasterfrom
single_arg_setdefault

Conversation

@bdraco
Copy link
Member

@bdraco bdraco commented May 4, 2025

fixes #1159

bdraco
bdraco commented May 4, 2025
View reviewed changes
@codspeed-hq
Copy link

codspeed-hq bot commented May 4, 2025 •
edited
Loading

CodSpeed Performance Report

Merging #1160 will degrade performances by 27.51%

Comparing single_arg_setdefault (ecd772f) with master (7770ff2)

Summary

❌ 12 regressions
✅ 232 untouched benchmarks

⚠️ Please fix the performance issues or acknowledge them on CodSpeed.

Benchmarks breakdown

Benchmark BASE HEAD Change
❌ test_iterate_multidict_items[case-insensitive-pure-python-module] 254.9 µs 329.8 µs -22.71%
❌ test_iterate_multidict_items[case-sensitive-pure-python-module] 180.2 µs 248.6 µs -27.51%
❌ test_multidict_extend_str[case-sensitive-pure-python-module] 55.1 ms 62 ms -11%
❌ test_multidict_popitem_str[case-insensitive-pure-python-module] 740.5 µs 813.7 µs -9%
❌ test_multidict_popitem_str[case-sensitive-pure-python-module] 518.8 µs 588.4 µs -11.82%
❌ test_items_view_less[case-insensitive-pure-python-module] 307.5 µs 383.3 µs -19.78%
❌ test_items_view_less[case-sensitive-pure-python-module] 233.2 µs 303.2 µs -23.08%
❌ test_items_view_less_or_equal[case-insensitive-pure-python-module] 304.7 µs 381.5 µs -20.13%
❌ test_items_view_less_or_equal[case-sensitive-pure-python-module] 230.9 µs 299.3 µs -22.86%
❌ test_items_view_or[case-sensitive-pure-python-module] 1.6 ms 1.7 ms -9.3%
❌ test_items_view_sub[case-insensitive-pure-python-module] 643.6 µs 731.5 µs -12%
❌ test_items_view_sub[case-sensitive-pure-python-module] 440.4 µs 514.4 µs -14.37%

@codecov
Copy link

codecov bot commented May 4, 2025 •
edited
Loading

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.62%. Comparing base (7770ff2) to head (ba8c950).
⚠️ Report is 73 commits behind head on master.

Additional details and impacted files 
@@           Coverage Diff           @@
##           master    #1160   +/-   ##
=======================================
  Coverage   98.62%   98.62%           
=======================================
  Files          27       27           
  Lines        3553     3558    +5     
  Branches      561      561           
=======================================
+ Hits         3504     3509    +5     
  Misses         17       17           
  Partials       32       32
Flag Coverage Δ
CI-GHA 98.62% <100.00%> (+<0.01%) ⬆️
MyPy 83.23% <100.00%> (+0.02%) ⬆️
pytest 100.00% <100.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

asvetlov
asvetlov approved these changes May 12, 2025
View reviewed changes
Copy link
Member

@asvetlov asvetlov left a comment

Choose a reason for hiding this comment

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

LGTM!

@psf-chronographer psf-chronographer bot added the bot:chronographer:provided There is a change note present in this PR label May 16, 2025
@bdraco bdraco marked this pull request as ready for review May 16, 2025 20:09
@bdraco bdraco requested a review from webknjaz as a code owner May 16, 2025 20:09
@bdraco bdraco merged commit fea968f into master May 16, 2025
63 checks passed
@bdraco bdraco deleted the single_arg_setdefault branch May 16, 2025 20:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@asvetlov asvetlov asvetlov approved these changes

@webknjaz webknjaz Awaiting requested review from webknjaz webknjaz is a code owner

Assignees

No one assigned

Labels

bot:chronographer:provided There is a change note present in this PR

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Segfault calling MultiDict().setdefault()

2 participants

@bdraco @asvetlov