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

Make python code compilable with a C++ compiler #49055

Closed
abalkin opened this issue Jan 2, 2009 · 13 comments
Closed

Make python code compilable with a C++ compiler #49055

abalkin opened this issue Jan 2, 2009 · 13 comments
Labels
type-feature A feature request or enhancement

Comments

@abalkin
Copy link
Member

abalkin commented Jan 2, 2009

BPO 4805
Nosy @malemburg, @abalkin, @serhiy-storchaka
Files
  • c++-patch.diff
  • c++-patch-2.diff
  • 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 = None
    created_at = <Date 2009-01-02.04:56:46.153>
    labels = ['type-feature']
    title = 'Make python code compilable with a C++ compiler'
    updated_at = <Date 2012-12-03.08:08:08.342>
    user = 'https://github.com/abalkin'

    bugs.python.org fields:

    activity = <Date 2012-12-03.08:08:08.342>
    actor = 'Arfrever'
    assignee = 'none'
    closed = False
    closed_date = None
    closer = None
    components = []
    creation = <Date 2009-01-02.04:56:46.153>
    creator = 'belopolsky'
    dependencies = []
    files = ['12527', '12560']
    hgrepos = []
    issue_num = 4805
    keywords = ['patch']
    message_count = 12.0
    messages = ['78756', '78764', '78821', '78822', '78823', '78928', '79130', '79162', '79182', '79194', '79201', '79248']
    nosy_count = 4.0
    nosy_names = ['lemburg', 'belopolsky', 'Arfrever', 'serhiy.storchaka']
    pr_nums = []
    priority = 'normal'
    resolution = None
    stage = 'patch review'
    status = 'open'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue4805'
    versions = ['Python 3.4']

    @abalkin
    Copy link
    Member Author

    abalkin commented Jan 2, 2009

    I am posting this patch mainly to support python-dev discussion on this
    topic. In the past (see r45330) it was possible to compile python core
    and standard library modules using a C++ compiler.

    According to Martin v. Löwis (bpo-4665), "It's not a requirement that
    the Python source code is compilable as C++. Only the header files
    should be thus compilable." However, I can see certain benefits from
    such requirement:

    1. It is hard to verify that header files are compilable if source code
      is not. With compilable source code, CC=g++ ./configure; make will
      supply an adequate test that does not require anything beyond a standard
      distribution.

    2. Arguably, C++ compliant code is more consistent and less error prone.
      For example, "new" is a really bad choice for a variable name regardless
      of being a C++ keyword, especially when used instead of prevailing "res"
      for a result of a function producing a PyObject. Even clearly redundant
      explicit casts of malloc return values arguably improve readability by
      reminding the type of the object that is being allocated.

    3. Compiling with C++ may reveal actual coding errors that otherwise go unnoticed. For example, use of undefined PyLong_BASE_TWODIGITS_TYPE in Objects/longobject.c.

    4. Stricter type checking may promote use of specific types instead of
      void* which in turn may help optimizing compilers.

    5. Once achieved, C++ compilability is not that hard to maintain, but
      restoring it with patches like this one is hard because it requires
      review of changes to many unrelated files.

    @abalkin abalkin added the type-feature A feature request or enhancement label Jan 2, 2009
    @abalkin
    Copy link
    Member Author

    abalkin commented Jan 2, 2009

    A related question discussed on python-dev is whether extern "C" {}
    wrappers should ever be used in .c files. I argue that the answer is "no"
    even if C++ compilability is desired.

    The new patch eliminates several uses of extern "C" {} in .c files while
    preserving C++ compilability. This is achieved by including proper header
    files instead of declaring external functions in .c files and in some
    cases adding declarations of functions that are used outside of the files
    they are defined in in the appropriate header files.

    @malemburg
    Copy link
    Member

    Moving declarations into header files is not really in line with the way
    Python developers use header files:

    We usually only put code into header files that is meant for public use.

    Buy putting declarations into the header files without additional
    warning, you implicitly document them and make them usable in
    non-interpreter code.

    @malemburg
    Copy link
    Member

    Also note that by removing the extern "C" declarations, you not only
    change the exported symbol names of functions, but also those of
    exported globals.

    Those would also have to get declared in the header files, to prevent
    their names from being mangled (causing the exported C API to change).

    @malemburg
    Copy link
    Member

    The added type casts are useful to have - even outside the context of
    the idea behind the patch.

    @abalkin
    Copy link
    Member Author

    abalkin commented Jan 3, 2009

    On Fri, Jan 2, 2009 at 10:54 AM, Marc-Andre Lemburg
    <[email protected]> wrote:

    Marc-Andre Lemburg <[email protected]> added the comment:

    Also note that by removing the extern "C" declarations, you not only
    change the exported symbol names of functions, but also those of
    exported globals.

    What are " exported globals" other than "exported symbol names of
    functions"? AFAIK, C++ does not mangle non-function symbols.

    Those would also have to get declared in the header files, to prevent
    their names from being mangled (causing the exported C API to change).

    I believe .c files should only contain static functions and functions
    that are declared in an included header file. If a function that is
    not advertised in a header, it is not part of API and it is a fair
    game to mangle it. The only exception is the module init functions
    that are part of the ABI rather than API.

    @malemburg
    Copy link
    Member

    On 2009-01-03 04:38, Alexander Belopolsky wrote:

    Alexander Belopolsky <[email protected]> added the comment:

    On Fri, Jan 2, 2009 at 10:54 AM, Marc-Andre Lemburg
    <[email protected]> wrote:
    > Marc-Andre Lemburg <[email protected]> added the comment:
    >
    > Also note that by removing the extern "C" declarations, you not only
    > changes the exported symbol names of functions, but also those of
    > exported globals.
    >
    What are " exported globals" other than "exported symbol names of
    functions"? AFAIK, C++ does not mangle non-function symbols.

    GCC doesn't appear to do so, but there's no guarantee that other
    C++ compilers won't touch these symbols:

    http://en.wikipedia.org/wiki/Name_mangling

    > Those would also have to get declared in the header files, to prevent
    > their names from being mangled (causing the exported C API to change).

    I believe .c files should only contain static functions and functions
    that are declared in an included header file. If a function that is
    not advertised in a header, it is not part of API and it is a fair
    game to mangle it. The only exception is the module init functions
    that are part of the ABI rather than API.

    That's right, but I was referring to non-function globals.
    These would have to be declared in the header files as well to
    prevent their names from being mangled.

    OTOH, those globals will often not be accessed directly from other
    object files - only through functions providing an interface to
    them. Still, this would have to be checked.

    @malemburg
    Copy link
    Member

    On 2009-01-05 13:03, Marc-Andre Lemburg wrote:
    > Marc-Andre Lemburg <[email protected]> added the comment:
    > 
    > On 2009-01-03 04:38, Alexander Belopolsky wrote:
    >> Alexander Belopolsky <[email protected]> added the comment:
    >>
    >> On Fri, Jan 2, 2009 at 10:54 AM, Marc-Andre Lemburg
    >> <[email protected]> wrote:
    >>> Marc-Andre Lemburg <mal@egenix.com> added the comment:
    >>>
    >>> Also note that by removing the extern "C" declarations, you not only
    >>> changes the exported symbol names of functions, but also those of
    >>> exported globals.
    >>>
    >> What are " exported globals" other than "exported symbol names of
    >> functions"?  AFAIK, C++ does not mangle non-function symbols.
    > 
    > GCC doesn't appear to do so, but there's no guarantee that other
    > C++ compilers won't touch these symbols:
    > 
    > http://en.wikipedia.org/wiki/Name_mangling

    Issue bpo-4846 is a good example of a situation where such name mangling
    causes problems even for non-function symbols.

    @abalkin
    Copy link
    Member Author

    abalkin commented Jan 5, 2009

    On Mon, Jan 5, 2009 at 11:43 AM, Marc-Andre Lemburg
    <[email protected]> wrote:
    ..

    > GCC doesn't appear to do so, but there's no guarantee that other
    > C++ compilers won't touch these symbols:
    >
    > http://en.wikipedia.org/wiki/Name_mangling

    Issue bpo-4846 is a good example of a situation where such name mangling
    causes problems even for non-function symbols.

    You are right, I did not know that fact about MS compilers. I am not
    sure what this means to the isue of removing extern "C" from the .c
    files, though. Note that properly declared in header files global
    symbols will not be affected, but only semi-private vars such as for
    example allocs counters in Objects/object.c.

    The allocs counters (tuple_zero_allocs, fast_tuple_allocs,
    quick_int_allocs, quick_neg_int_allocs) present a case where it is
    really hard to justify a change that is only motivated by C++
    compilability. Note that currently they are not getting extern "C"
    at the point of definition (Objects/tupleobject.c and
    Objects/intobject.c) but do at the point of declaration
    (Objects/object.c). Moving them to a header file would require
    renaming with a _Py_ prefix. Affected applications are really
    esoteric: MS C++ compilation with -DCOUNT_ALLOCS.

    I find it hard to get motivated to do a more thorough review of the
    code searching for affected non-function symbols. My original
    motivation was just the curiosity as to why extern "C" were added to
    .c files. I got my questions answered and I believe these
    declarations serve no valid purpose, particularly inside the files
    that no longer valid C++.

    I see little to be gained in refining the patch further to support
    non-g++ compilers. It does not look like there is much interest in
    C++ compilability to begin with. Despite my posting to c++-sig
    mailing list, no one has subscribed to this issue so far. Maybe we
    should ask on the python-list as well.

    @malemburg
    Copy link
    Member

    On 2009-01-05 19:55, Alexander Belopolsky wrote:

    The allocs counters (tuple_zero_allocs, fast_tuple_allocs,
    quick_int_allocs, quick_neg_int_allocs) present a case where it is
    really hard to justify a change that is only motivated by C++
    compilability. Note that currently they are not getting extern "C"
    at the point of definition (Objects/tupleobject.c and
    Objects/intobject.c) but do at the point of declaration
    (Objects/object.c). Moving them to a header file would require
    renaming with a _Py_ prefix. Affected applications are really
    esoteric: MS C++ compilation with -DCOUNT_ALLOCS.

    For completeness, all exported symbols in Python should have a _Py_
    prefix, even if they only get exported in certain debug builds.

    I find it hard to get motivated to do a more thorough review of the
    code searching for affected non-function symbols. My original
    motivation was just the curiosity as to why extern "C" were added to
    .c files. I got my questions answered and I believe these
    declarations serve no valid purpose, particularly inside the files
    that no longer valid C++.

    Like I mentioned earlier on: those declarations did serve a purpose
    for early MS VC++ versions (at least AFAIR). It may well be the case
    that they are no longer needed nowadays, but then they don't hurt
    much either.

    Given that you can build Python as library on Unix and as DLL on
    Windows, there doesn't appear to be any need to actually be able
    to build Python itself using a C++ compiler. Simply using the
    header files and linking against those libraries should do the
    trick in most cases.

    @abalkin
    Copy link
    Member Author

    abalkin commented Jan 5, 2009

    On Mon, Jan 5, 2009 at 2:58 PM, Marc-Andre Lemburg
    <[email protected]> wrote:
    ..

    For completeness, all exported symbols in Python should have a _Py_
    prefix, even if they only get exported in certain debug builds.

    I actually agree, but I felt that doing this as a part of this patch
    would make it even less likely to be accepted. There is another
    change that needs to be done to the alloc counts - namely changing the
    type from int to Py_ssize_t and %d to %zd in print formats. I will
    submit that as a separate issue. (See bpo-4850.)

    The only downside of having them is that a #ifdef __cplusplus
    instruction strongly suggests that a file is intended to be valid C++,
    which is currently not the case.

    Given that you can build Python as library on Unix and as DLL on
    Windows, there doesn't appear to be any need to actually be able
    to build Python itself using a C++ compiler. Simply using the
    header files and linking against those libraries should do the
    trick in most cases.

    So what is your position on the proposed patch? Is it worthwhile to
    track down the remaining symbols that may be affected by removal of
    extern "C" from .c files? What is your opinion on the original patch
    (c++-patch.diff) which restores C++ compilability but does not touch
    these declarations?

    I think using C++ as a lint variant from time to time is a good
    exercise to catch some corner issues as I hope this patch
    demonstrates. I don't think this should be a requirement on every
    submission, but once an effort is made to restore C++ compilability,
    such changes should be applied unless there are valid concerns against
    them.

    @malemburg
    Copy link
    Member

    On 2009-01-05 22:06, Alexander Belopolsky wrote:

    Alexander Belopolsky <[email protected]> added the comment:
    > Given that you can build Python as library on Unix and as DLL on
    > Windows, there doesn't appear to be any need to actually be able
    > to build Python itself using a C++ compiler. Simply using the
    > header files and linking against those libraries should do the
    > trick in most cases.

    So what is your position on the proposed patch? Is it worthwhile to
    track down the remaining symbols that may be affected by removal of
    extern "C" from .c files? What is your opinion on the original patch
    (c++-patch.diff) which restores C++ compilability but does not touch
    these declarations?

    I think using C++ as a lint variant from time to time is a good
    exercise to catch some corner issues as I hope this patch
    demonstrates. I don't think this should be a requirement on every
    submission, but once an effort is made to restore C++ compilability,
    such changes should be applied unless there are valid concerns against
    them.

    I agree with using C++ compatibility as additional means of checking
    for correctness of the code. The type casts you have added in the
    patch should definitely make it into the trunk.

    Making sure that all exported private symbols get a _Py prefix and
    a declaration in the header files that adds a comment explaining their
    private nature is also a good idea.

    I'm not sure about removing the extern "C" declarations altogether.
    We'd need further testing with non-G++ compilers to see whether we
    still need them or not. With the above fixes, I doubt that we still
    need them nowadays.

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    @kumaraditya303
    Copy link
    Contributor

    Duplicate of #91321

    @kumaraditya303 kumaraditya303 marked this as a duplicate of #91321 Jun 18, 2022
    @kumaraditya303 kumaraditya303 closed this as not planned Won't fix, can't repro, duplicate, stale Jun 18, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    type-feature A feature request or enhancement
    Projects
    None yet
    Development

    No branches or pull requests

    3 participants