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

Argument Clinic's [python input] blocks do not support empty lines in comments #128152

Closed
picnixz opened this issue Dec 21, 2024 · 6 comments · Fixed by #128464
Closed

Argument Clinic's [python input] blocks do not support empty lines in comments #128152

picnixz opened this issue Dec 21, 2024 · 6 comments · Fixed by #128464
Labels
topic-argument-clinic type-bug An unexpected behavior, bug, or error

Comments

@picnixz
Copy link
Member

picnixz commented Dec 21, 2024

Bug report

Bug description:

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

/*[python input]
# Line 1
#
# Line 3
[python start generated code]*/

This results in an assertion error:

$ python ./Tools/clinic/clinic.py --force --make --exclude Lib/test/clinic.test.c --srcdir .
Traceback (most recent call last):
  File "/lib/python/cpython/./Tools/clinic/clinic.py", line 11, in <module>
    main()
  File "/lib/python/cpython/Tools/clinic/libclinic/cli.py", line 226, in main
    run_clinic(parser, args)
  File "/lib/python/cpython/Tools/clinic/libclinic/cli.py", line 205, in run_clinic
    parse_file(path,
  File "/lib/python/cpython/Tools/clinic/libclinic/cli.py", line 84, in parse_file
    cooked = clinic.parse(raw)
             ^^^^^^^^^^^^^^^^^
  File "/lib/python/cpython/Tools/clinic/libclinic/app.py", line 186, in parse
    for block in self.block_parser:
                 ^^^^^^^^^^^^^^^^^
  File "/lib/python/cpython/Tools/clinic/libclinic/block_parser.py", line 132, in __next__
    return_value = self.parse_clinic_block(self.dsl_name)
                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/lib/python/cpython/Tools/clinic/libclinic/block_parser.py", line 194, in parse_clinic_block
    line = self._line()
           ^^^^^^^^^^^^
  File "/lib/python/cpython/Tools/clinic/libclinic/block_parser.py", line 155, in _line
    self.language.parse_line(line)
  File "/lib/python/cpython/Tools/clinic/libclinic/clanguage.py", line 70, in parse_line
    self.cpp.writeline(line)
  File "/lib/python/cpython/Tools/clinic/libclinic/cpp.py", line 139, in writeline
    assert line
           ^^^^
AssertionError
make: *** [Makefile:1055: clinic] Error 1

CPython versions tested on:

CPython main branch

Operating systems tested on:

No response

Linked PRs

@picnixz picnixz added type-bug An unexpected behavior, bug, or error topic-argument-clinic labels Dec 21, 2024
@picnixz picnixz changed the title Argument Clinic's [python input] blocks do not supported nested C comments Argument Clinic's [python input] blocks do not support empty lines in comments Dec 21, 2024
@picnixz
Copy link
Member Author

picnixz commented Dec 21, 2024

I've had a quick look at the failure and this happens before we've detected the [python input] block. Similarly, #128153 faces the same issue, namely Clinic assumes that we're still processing a C file and doesn't check for the desired DSL.

@erlend-aasland I don't really know how to fix this and it's not really an issue since I can just write some fake characters to simulate an empty line (e.g., # ::) but I'd like to know if first detecting the DSL before attempting to parse it would be a good alternative or if the current design of AC is just too intricate for this feature.

@StanFromIreland
Copy link
Contributor

I've opened a PR for this issue. I think the problem was that empty lines were breaking the logic so if they don't reach it there should be no more error.

@picnixz
Copy link
Member Author

picnixz commented Dec 21, 2024

As I said on the PR, I'm not sure this is the best approach. The "empty lines" that are detected at this stage are actually lines of the form <space>#</space>, so it's not entirely clear whether they should be ignored at this point. The reason is that we first parse a file before deciding whether this new line is part of a python input block. If this is the case, we should switch to another parsing method (for instance, I haven't actually checked what happens if I write #if defined(...) inside the Python input clinic block).

@erlend-aasland
Copy link
Contributor

This has nothing to do with [python input]; you'll get the same error with [clinic input]:

[clinic input] repro
$ cat > buggy.c
/*[clinic input]
# comment
#
[clinic start generated code]*/
$ ./Tools/clinic/clinic.py buggy.c 
Traceback (most recent call last):
  File "/Users/erlend.aasland/src/python/main/./Tools/clinic/clinic.py", line 11, in <module>
    main()
    ~~~~^^
  File "/Users/erlend.aasland/src/python/main/Tools/clinic/libclinic/cli.py", line 226, in main
    run_clinic(parser, args)
    ~~~~~~~~~~^^^^^^^^^^^^^^
  File "/Users/erlend.aasland/src/python/main/Tools/clinic/libclinic/cli.py", line 218, in run_clinic
    parse_file(filename, output=ns.output,
    ~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^
               verify=not ns.force, limited_capi=ns.limited_capi)
               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/erlend.aasland/src/python/main/Tools/clinic/libclinic/cli.py", line 84, in parse_file
    cooked = clinic.parse(raw)
  File "/Users/erlend.aasland/src/python/main/Tools/clinic/libclinic/app.py", line 186, in parse
    for block in self.block_parser:
                 ^^^^^^^^^^^^^^^^^
  File "/Users/erlend.aasland/src/python/main/Tools/clinic/libclinic/block_parser.py", line 132, in __next__
    return_value = self.parse_clinic_block(self.dsl_name)
  File "/Users/erlend.aasland/src/python/main/Tools/clinic/libclinic/block_parser.py", line 194, in parse_clinic_block
    line = self._line()
  File "/Users/erlend.aasland/src/python/main/Tools/clinic/libclinic/block_parser.py", line 155, in _line
    self.language.parse_line(line)
    ~~~~~~~~~~~~~~~~~~~~~~~~^^^^^^
  File "/Users/erlend.aasland/src/python/main/Tools/clinic/libclinic/clanguage.py", line 70, in parse_line
    self.cpp.writeline(line)
    ~~~~~~~~~~~~~~~~~~^^^^^^
  File "/Users/erlend.aasland/src/python/main/Tools/clinic/libclinic/cpp.py", line 139, in writeline
    assert line
           ^^^^
AssertionError

@picnixz
Copy link
Member Author

picnixz commented Jan 3, 2025

Yes, that's why I'm asking if this is possible to pre-determine the DSL before parsing the C file (a two-pass parsing) so that python input blocks are parsed using the Python parser (namely, if we're in a python input block, we skip the special handling of # which assumes that we're parsing pre-processor guards). Because the error happens before this (I'm not sure it's possible)

@erlend-aasland
Copy link
Contributor

The problem seems to be in cpp.py's incorrect parsing of comments: it tries to parse directives inside comments. The fix is straightforward: ignore CPP directives inside C comments.

miss-islington pushed a commit to miss-islington/cpython that referenced this issue Jan 4, 2025
…ide C comments (pythonGH-128464)

(cherry picked from commit a4e773c)

Co-authored-by: Erlend E. Aasland <erlend@python.org>
erlend-aasland added a commit that referenced this issue Jan 4, 2025
…side C comments (GH-128464) (#128478)

(cherry picked from commit a4e773c)

Co-authored-by: Erlend E. Aasland <erlend@python.org>
srinivasreddy pushed a commit to srinivasreddy/cpython that referenced this issue Jan 6, 2025
srinivasreddy pushed a commit to srinivasreddy/cpython that referenced this issue Jan 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic-argument-clinic type-bug An unexpected behavior, bug, or error
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants