Fix peg_generator compiler warnings under MSVC #20405
Merged
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This fixes the following warnings:
The changes to
peg_extension.c
are pretty simple.For the tokenizer.c change, looking through the blame, it looks like
PyOS_Readline
was extern'd so it could be included in both the old Cpgen
executable and the interpreter:pgen
in bpo-35808, we still used to link withtokenizer.o
andmyreadline.o
: https://github.com/python/cpython/blob/3.7/Makefile.pre.in#L311)Unfortunately right now this causes a warning due when building the
peg_generator
module since it uses tokenizer.c as a source file. This causes a mismatch between the extern'dPyOS_Readline
and thePyAPI_FUNC
'd version of the functions.Since we're building an extension, this causes pyport.h to set the function as a
__declspec(dllimport)
which doesn't match the extern definition without the import declaration. See the page on the warning for more details: https://docs.microsoft.com/en-us/cpp/error-messages/compiler-warnings/compiler-warning-level-1-c4273?view=vs-2019As far as I know, the only supported way of using tokenizer.c is with a Python runtime (since the
#ifndef PGEN
guards were removed) so this extern should be safe to remove now.