Skip to content

Regression: name of PyType_Spec::slots conflicts with Qt macro #82188

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

Closed
LeslieGerman mannequin opened this issue Sep 2, 2019 · 7 comments
Closed

Regression: name of PyType_Spec::slots conflicts with Qt macro #82188

LeslieGerman mannequin opened this issue Sep 2, 2019 · 7 comments
Labels
3.9 only security fixes build The build process and cross-build

Comments

@LeslieGerman
Copy link
Mannequin

LeslieGerman mannequin commented Sep 2, 2019

BPO 38007
Nosy @terryjreedy, @vstinner, @benjaminp, @encukou, @serhiy-storchaka, @zooba, @LeslieGerman
PRs
  • bpo-38007: Added ts_ prefix to PyType_Spec struct members #15644
  • 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 = None
    closed_at = <Date 2019-09-09.14:23:46.202>
    created_at = <Date 2019-09-02.10:23:47.632>
    labels = ['build', '3.9']
    title = 'Regression: name of PyType_Spec::slots conflicts with Qt macro'
    updated_at = <Date 2019-10-22.08:35:09.342>
    user = 'https://github.com/LeslieGerman'

    bugs.python.org fields:

    activity = <Date 2019-10-22.08:35:09.342>
    actor = 'petr.viktorin'
    assignee = 'none'
    closed = True
    closed_date = <Date 2019-09-09.14:23:46.202>
    closer = 'steve.dower'
    components = []
    creation = <Date 2019-09-02.10:23:47.632>
    creator = 'Leslie'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 38007
    keywords = ['patch']
    message_count = 7.0
    messages = ['350977', '351275', '351320', '351361', '351478', '351502', '351796']
    nosy_count = 7.0
    nosy_names = ['terry.reedy', 'vstinner', 'benjamin.peterson', 'petr.viktorin', 'serhiy.storchaka', 'steve.dower', 'Leslie']
    pr_nums = ['15644']
    priority = 'normal'
    resolution = 'rejected'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'compile error'
    url = 'https://bugs.python.org/issue38007'
    versions = ['Python 3.9']

    @LeslieGerman
    Copy link
    Mannequin Author

    LeslieGerman mannequin commented Sep 2, 2019

    In Feb 20, 2006, in commit c255c7b the bug https://bugs.python.org/issue1086854 had been fixed.

    In Dec 3, 2010, in commit 4d0d471 basically the same issue has been introduced again, because it added a "slots" member to the added PyType_Spec struct.

    The proposed solution is the same like in the earlier commit, which is explained in https://bugs.python.org/msg23759 :
    protect PyType_Spec struct members with prefix:
    ts_name, ts_doc, ts_basicsize, ts_itemsize, ts_flags, ts_slots

    @LeslieGerman LeslieGerman mannequin added 3.7 (EOL) end of life 3.8 (EOL) end of life 3.9 only security fixes build The build process and cross-build labels Sep 2, 2019
    @terryjreedy
    Copy link
    Member

    My initial thought is similar to that of Michael Hudson in msg23757, that Python should not be hostage to foreign code processors. I also note that the 2010 patch is by Martin Loewis, who was part of the 2006 discussion, and that the 2010 patch was about the stable public ABI, which should not be changed. But I am not familiar with C details at all.

    @terryjreedy terryjreedy removed 3.7 (EOL) end of life 3.8 (EOL) end of life labels Sep 7, 2019
    @serhiy-storchaka
    Copy link
    Member

    This is a major breaking change. It may need changing the Python version to 4.0.

    I suggest you to not use global macros without prefixes and undefine the conflicting one before using the Python C API.

    @vstinner
    Copy link
    Member

    vstinner commented Sep 9, 2019

    I don't understand this issue, it mostly contains reference to other bugs. Can someone please try to elaborate the issue?

    "Regression: name of PyType_Spec::slots conflicts with Qt macro"

    What is "::slots"? Is it C++ syntax?

    I don't understand how "slots" name can be an issue: it's not a global variable, but a structure member name.

    @zooba
    Copy link
    Member

    zooba commented Sep 9, 2019

    Yes, even as a struct member name (at least in the definition), the C preprocessor will substitute a macro. It sounds like Qt defines a macro "slots", and so the definition of the struct will be broken.

    Unfortunately, this would be a more significant change than removing the deprecated tp_print field, which we eventually decided not to do because of the compatibility impact. So I can confidently say we won't be able to do this one either.

    @zooba zooba closed this as completed Sep 9, 2019
    @benjaminp
    Copy link
    Contributor

    (You might also argue that Qt should be the ones fixing their namespacing.)

    @LeslieGerman
    Copy link
    Mannequin Author

    LeslieGerman mannequin commented Sep 11, 2019

    Some history and explanation of this problem:

    • Qt extends C++ with the following keywords:
      "slots", "signals", "emit"
    • This extension is compiled with a Meta-Object-Compiler (MOC) into standard C++.
    • Since code using Qt extensions (i.e. headers) generally used "mixed" with standard C++ code, "standard" compiler must be able to compile the Qt specific part, too
      -> the extension keywords are declared as empty macros (i.e. #define slots)

    The consequence is that if Qt-based headers are used together with any 3rd party headers, the 3rd party code cannot contain the extension keywords (slots, signals, emit) as C/C++ tokens, because the preprocessor will "delete" them as a result of macro expansion, since the keywords are defined as empty macros.
    ->
    The code won't compile because of syntax errors.

    This caused bug https://bugs.python.org/issue1086854 , which was fixed in c255c7b

    The fix renamed the "slots" struct member in Python, thus the conflict was resolved.

    Note: the Qt library is, like Python, old, and used in a huge number of projects and products. E.g. Qt is the base platform for KDE development. It's a matter of point of view, which of the two, Qt or Python should be "fixed".

    In my PR I used the same solution: renaming the conflicting member.
    -------------

    Since this time this solution was not welcome, and obviously I'm not the first facing this issue, I was searching for another solutions.

    I found that Qt has already provided a solution for this problem.

    There are 2 macros: QT_NO_SIGNALS_SLOTS_KEYWORDS and QT_NO_KEYWORDS , which "turn off" defining the extension keywords as macros.
    Instead the QT_SLOTS, Q_SIGNALS macros should be used, which very likely do not interfere with any 3rd party library...

    Though, these are "official" Qt macros, not very well documented: https://bugreports.qt.io/browse/QTBUG-70564 :)

    So by defining QT_NO_KEYWORDS I could resolve this whole issue.

    Thanks guys for your patience and attetion!
    :)

    @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
    Labels
    3.9 only security fixes build The build process and cross-build
    Projects
    None yet
    Development

    No branches or pull requests

    5 participants