Skip to content
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.

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

Depreciated MACRO of copysign for MSVC #86286

Closed
ejguan mannequin opened this issue Oct 22, 2020 · 7 comments
Closed

Depreciated MACRO of copysign for MSVC #86286

ejguan mannequin opened this issue Oct 22, 2020 · 7 comments
Assignees
@zooba
Labels
3.8 only security fixes 3.9 only security fixes 3.10 only security fixes build The build process and cross-build OS-windows topic-C-API

Comments

@ejguan
Copy link
Mannequin

ejguan mannequin commented Oct 22, 2020

BPO 42120
Nosy @pfmoore, @ronaldoussoren, @tjguk, @zware, @zooba, @miss-islington, @ejguan
PRs
  • bpo-42120: Remove macro defining copysign to _copysign on Windows #23326
  • [3.8] bpo-42120: Remove macro defining copysign to _copysign on Windows (GH-23326) #23330
  • [3.9] bpo-42120: Remove macro defining copysign to _copysign on Windows (GH-23326) #23331
    		•

    Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = 'https://github.com/zooba'
closed_at = <Date 2020-11-21.01:10:32.360>
created_at = <Date 2020-10-22.19:08:50.650>
labels = ['3.8', '3.9', '3.10', 'expert-C-API', 'build', 'OS-windows']
title = 'Depreciated MACRO of copysign for MSVC'
updated_at = <Date 2020-11-21.01:10:32.360>
user = 'https://github.com/ejguan'

    bugs.python.org fields:

    activity = <Date 2020-11-21.01:10:32.360>
actor = 'steve.dower'
assignee = 'steve.dower'
closed = True
closed_date = <Date 2020-11-21.01:10:32.360>
closer = 'steve.dower'
components = ['Windows', 'C API']
creation = <Date 2020-10-22.19:08:50.650>
creator = 'ejguan'
dependencies = []
files = []
hgrepos = []
issue_num = 42120
keywords = ['patch']
message_count = 7.0
messages = ['379323', '379325', '379570', '381174', '381181', '381186', '381217']
nosy_count = 7.0
nosy_names = ['paul.moore', 'ronaldoussoren', 'tim.golden', 'zach.ware', 'steve.dower', 'miss-islington', 'ejguan']
pr_nums = ['23326', '23330', '23331']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'compile error'
url = 'https://bugs.python.org/issue42120'
versions = ['Python 3.8', 'Python 3.9', 'Python 3.10']
    @ejguan
    Copy link
    Mannequin Author

    ejguan mannequin commented Oct 22, 2020

    At

    cpython/PC/pyconfig.h

    Line 196 in c60394c

    #define copysign _copysign
    , the MACRO converts copysign to _copysign. This MACRO is defined 13 years ago and it's really dangerous to define a lower-letter MACRO at this low level.
    Currently, MSVC is supporting copysign (https://docs.microsoft.com/en-us/cpp/c-runtime-library/reference/copysign-copysignf-copysignl-copysign-copysignf-copysignl?view=vs-2019).
    I am requesting to remove this MACRO, as my customized copysign API will also be converted to _copysign on MSVC platform.

    @ejguan ejguan mannequin added 3.8 only security fixes topic-C-API build The build process and cross-build 3.9 only security fixes 3.10 only security fixes labels Oct 22, 2020
    @zooba
    Copy link
    Member

    zooba commented Oct 22, 2020

    Sounds good, but I don't think we can backport it. It's not _our_ public API, but it's still going to impact compiling modules.

    Also, looking at the docs link above, copysign() is for floats and _copysign() is for doubles, which means it's an actual functional change. If POSIX/libc copysign() is floats, it's probably fine for a 3.10 change, but if we need to preserve the type then it needs to become _Py_copysign throughout.

    @zooba zooba removed 3.8 only security fixes 3.9 only security fixes labels Oct 22, 2020
    @ronaldoussoren
    Copy link
    Contributor

    The standard is for copysign to work on doubles, with copysignf as the same functionality for floats.

    https://en.cppreference.com/w/c/numeric/math/copysign

    @zooba
    Copy link
    Member

    zooba commented Nov 16, 2020

    Turns out a straight delete is fine, as both names are defined in UCRT. I think it's safe to backport as well, since it's only going to affect other people's code poorly (and they'll need to recompile to see any change anyway).

    @zooba zooba added 3.8 only security fixes 3.9 only security fixes labels Nov 16, 2020
    @zooba zooba self-assigned this Nov 16, 2020
    @zooba zooba added 3.8 only security fixes 3.9 only security fixes labels Nov 16, 2020
    @zooba zooba self-assigned this Nov 16, 2020
    @zooba
    Copy link
    Member

    zooba commented Nov 16, 2020

    New changeset 9cc9e27 by Steve Dower in branch 'master':
    bpo-42120: Remove macro defining copysign to _copysign on Windows (GH-23326)
    9cc9e27

    @miss-islington
    Copy link
    Contributor

    New changeset 4f54ca0 by Miss Islington (bot) in branch '3.8':
    bpo-42120: Remove macro defining copysign to _copysign on Windows (GH-23326)
    4f54ca0

    @miss-islington
    Copy link
    Contributor

    New changeset 2c38e49 by Miss Islington (bot) in branch '3.9':
    [3.9] bpo-42120: Remove macro defining copysign to _copysign on Windows (GH-23326) (GH-23331)
    2c38e49

    @zooba zooba closed this as completed Nov 21, 2020
    @zooba zooba closed this as completed Nov 21, 2020
    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Assignees

    @zooba zooba

    Labels
    3.8 only security fixes 3.9 only security fixes 3.10 only security fixes build The build process and cross-build OS-windows topic-C-API
    Projects
    None yet
    Milestone
    No milestone
    Development

    No branches or pull requests
    4 participants
    @ronaldoussoren @zooba @zware @miss-islington