Skip to content

bpo-39684 : Combine two if/thens. Squash uninit var warning. #18565

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 1 commit into from
Feb 21, 2020
Merged

bpo-39684 : Combine two if/thens. Squash uninit var warning. #18565

merged 1 commit into from
Feb 21, 2020

Conversation

petdance
Copy link
Contributor

@petdance petdance commented Feb 20, 2020 •
edited by bedevere-bot
Loading

These two code if/thens can be combined

if (ready) {
    kind = PyUnicode_KIND(self);
    data = PyUnicode_DATA(self);
}
else {
    wstr = _PyUnicode_WSTR(self);
}

Py_UCS4 ch;
if (ready) {
    ch = PyUnicode_READ(kind, data, 0);
}
else {
    ch = wstr[0];
}

This is a replacement for previous PR #18557.

https://bugs.python.org/issue39684

@codecov
Copy link

codecov bot commented Feb 20, 2020 •
edited
Loading

Codecov Report

Merging #18565 into master will decrease coverage by 0.05%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #18565      +/-   ##
==========================================
- Coverage   82.11%   82.06%   -0.06%     
==========================================
  Files        1956     1955       -1     
  Lines      589464   584012    -5452     
  Branches    44458    44458              
==========================================
- Hits       484059   479279    -4780     
+ Misses      95754    95108     -646     
+ Partials     9651     9625      -26
Impacted Files Coverage Δ
Lib/distutils/tests/test_bdist_rpm.py 30.00% <0.00%> (-65.00%) ⬇️
Lib/distutils/command/bdist_rpm.py 7.63% <0.00%> (-56.88%) ⬇️
Modules/_decimal/libmpdec/umodarith.h 80.76% <0.00%> (-19.24%) ⬇️
Lib/test/test_urllib2net.py 76.92% <0.00%> (-13.85%) ⬇️
Lib/test/test_smtpnet.py 78.57% <0.00%> (-7.15%) ⬇️
Lib/ftplib.py 63.85% <0.00%> (-6.06%) ⬇️
Lib/test/test_ftplib.py 87.11% <0.00%> (-4.72%) ⬇️
Tools/scripts/db2pickle.py 17.82% <0.00%> (-3.97%) ⬇️
Tools/scripts/pickle2db.py 16.98% <0.00%> (-3.78%) ⬇️
Lib/test/test_socket.py 71.94% <0.00%> (-3.77%) ⬇️
... and 323 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f2ee21d...91f5d56. Read the comment docs.

@shihai1991
Copy link
Member

shihai1991 commented Feb 20, 2020 •
edited
Loading

Hi, Andy. I had the same idea as you. But I give up this idea because those code of ch looks like rigorous too(Neutral view).

    if (ready) {
        ch = PyUnicode_READ(kind, data, 0);
    }
    else {
        ch = wstr[0];
    }
    if (!_PyUnicode_IsXidStart(ch) && ch != 0x5F /* LOW LINE */) {
        return 0;
    }

@petdance
Copy link
Contributor Author

I give up this idea because those code of ch looks like rigorous too(Neutral view).

I'm sorry, I'm not sure what you're saying here.

@shihai1991
Copy link
Member

I give up this idea because those code of ch looks like rigorous too(Neutral view).

I'm sorry, I'm not sure what you're saying here.

Oh, sorry for my poor English. I mean that putting the operation code of ch togther makes it more logical(my humble opinion). I am not sure I express my thought clearly or not :(

I remember victor edit those code recently, so cc @vstinner

@benjaminp benjaminp merged commit 933fc53 into python:master Feb 21, 2020
@petdance petdance deleted the bpo-39684-B branch February 21, 2020 19:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
skip news
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@petdance @shihai1991 @benjaminp @the-knights-who-say-ni @bedevere-bot