Skip to content

Argument Clinic's [python input] blocks do not support /* in literal strings #128153

Closed as not planned

Description

@picnixz

Bug report

Bug description:

The following is not a valid Python input clinic but should be:

/*[python input]
print("/* this is a comment *\/")
[python start generated code]*/

Note that the inner C comment should be *\/ because otherwise IDEs will consider the block comment to be closed resulting in syntax errors (for the C file). Another example is just:

/*[python input]
x = "/*"
[python start generated code]*/

Both result in the following error (I've just added the AC directive in the first file I had opened):

$ python3 ./Tools/clinic/clinic.py --force --make --exclude Lib/test/clinic.test.c --srcdir .
Error in file 'Modules/sha1module.c' on line 270:
Nested block comment!
make: *** [Makefile:1055: clinic] Error 1

For now the workaround is to break /* into multiple strings: x = "/" + "*" for instance.

CPython versions tested on:

CPython main branch

Operating systems tested on:

No response

Activity

  1. changed the title [-] Argument Clinic's [python input] blocks do not support `/*` in literal strings[/-] [+] Argument Clinic's `[python input]` blocks do not support `/*` in literal strings[/+] on Dec 21, 2024
  2. erlend-aasland commented on Jan 3, 2025

    @erlend-aasland
    Contributor

    The following is not a valid Python input clinic but should be:

    /*[python input]
    print("/* this is a comment *\/")
    [python start generated code]*/

    Should it? This will generate a C compiler warning:

    $ gcc -c buggy.c
    buggy.c:2:8: warning: '/*' within block comment [-Wcomment]
        2 | print("/* this is a comment *\/")
          |        ^
    1 warning generated.
    

    Would it not be better to use C++ style comments for these cases?

    /*[python input]
    print("// this comment is nicer both for Python and the C pre-processor")
    [python start generated code]*/
    
  3. picnixz commented on Jan 3, 2025

    @picnixz
    MemberAuthor

    Should it? This will generate a C compiler warning:

    Oh I'm surprised about this. I'll need to check on my side if adding \/* would be fine or not for the C compiler. Though, I think it would still not be fine for clinic since the substring /* is present.

    I'm going to sleep now so I'll need to think about feature this is a bug or not more tomorrow.

    Would it not be better to use C++ style comments for these cases?

    I think I can use // comments but it was mostly a question of style (I usually reserve /* for external documentation (like a Doxygen thing) and // for internal documentation). But I can make an exception for clinic (see https://github.com/python/cpython/pull/125011/files#diff-7bb6f3c67c8fec78f79fc7f5f74f26aa808d97c5e2ce5a538292056f8eb46b8aR150 for its usage).

  4. erlend-aasland commented on Jan 3, 2025

    @erlend-aasland
    Contributor

    For the linked use case, // is definitely fine. Moreover, we don't use Doxygen, and there are no style recommendations regarding old-style or new-style C comments. You actually can't make an exception; there is no rule :)

    Suggesting to rebrand this issue as a feature request, and I'm aligned to reject that feature.

  5. picnixz commented on Jan 3, 2025

    @picnixz
    MemberAuthor

    But I can make an exception for clinic

    I mean an exception to my own guidelines for me :')

    Suggesting to rebrand this issue as a feature request, and I'm aligned to reject that feature.

    I'm fine with a rejection since the workaround isn't really an issue (the only reason why /* is needed is if you want to generate a multi-line macro with comments inside since // are not allowed in this case AFAIK). Since I don't have a use-case for now, let's close it for now.

  6. added
    type-featureA feature request or enhancement
    and removed
    type-bugAn unexpected behavior, bug, or error
    on Jan 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions