Skip to content

Commit

Permalink
[mypyc] Always emit warnings (#14451)
Browse files Browse the repository at this point in the history
For example the warning for "treating generator comprehension as list"
doesn't get printed unless there were errors too. This was due to the
fact mypyc/build.py was only checking errors.num_errors to decide
whether to print messages to STDOUT.

To be honest, the generate_c() logic was pretty messy, so I broke out
the message printing logic into a separate helper function and made
liberal use of early exits.

Fixes mypyc/mypyc#873 (comment).
  • Loading branch information
ichard26 authored Jan 16, 2023
1 parent e9f5858 commit e88f4a4
Show file tree
Hide file tree
Showing 3 changed files with 35 additions and 27 deletions.
51 changes: 24 additions & 27 deletions mypyc/build.py
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,15 @@ def fail(message: str) -> NoReturn:
sys.exit(message)


def emit_messages(options: Options, messages: list[str], dt: float, serious: bool = False) -> None:
# ... you know, just in case.
if options.junit_xml:
py_version = f"{options.python_version[0]}_{options.python_version[1]}"
write_junit_xml(dt, serious, messages, options.junit_xml, py_version, options.platform)
if messages:
print("\n".join(messages))


def get_mypy_config(
mypy_options: list[str],
only_compile_paths: Iterable[str] | None,
Expand Down Expand Up @@ -191,47 +200,35 @@ def generate_c(
"""
t0 = time.time()

# Do the actual work now
serious = False
result = None
try:
result = emitmodule.parse_and_typecheck(
sources, options, compiler_options, groups, fscache
)
messages = result.errors
except CompileError as e:
messages = e.messages
if not e.use_stdout:
serious = True
emit_messages(options, e.messages, time.time() - t0, serious=(not e.use_stdout))
sys.exit(1)

t1 = time.time()
if result.errors:
emit_messages(options, result.errors, t1 - t0)
sys.exit(1)

if compiler_options.verbose:
print(f"Parsed and typechecked in {t1 - t0:.3f}s")

if not messages and result:
errors = Errors()
modules, ctext = emitmodule.compile_modules_to_c(
result, compiler_options=compiler_options, errors=errors, groups=groups
)

if errors.num_errors:
messages.extend(errors.new_messages())

errors = Errors()
modules, ctext = emitmodule.compile_modules_to_c(
result, compiler_options=compiler_options, errors=errors, groups=groups
)
t2 = time.time()
emit_messages(options, errors.new_messages(), t2 - t1)
if errors.num_errors:
# No need to stop the build if only warnings were emitted.
sys.exit(1)

if compiler_options.verbose:
print(f"Compiled to C in {t2 - t1:.3f}s")

# ... you know, just in case.
if options.junit_xml:
py_version = f"{options.python_version[0]}_{options.python_version[1]}"
write_junit_xml(
t2 - t0, serious, messages, options.junit_xml, py_version, options.platform
)

if messages:
print("\n".join(messages))
sys.exit(1)

return ctext, "\n".join(format_modules(modules))


Expand Down
6 changes: 6 additions & 0 deletions mypyc/test-data/commandline.test
Original file line number Diff line number Diff line change
Expand Up @@ -223,3 +223,9 @@ def h(arg: str) -> None:
@a.register
def i(arg: Foo) -> None:
pass

[case testOnlyWarningOutput]
# cmd: test.py

[file test.py]
names = (str(v) for v in [1, 2, 3]) # W: Treating generator comprehension as list
5 changes: 5 additions & 0 deletions mypyc/test/test_commandline.py
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,11 @@ def run_case(self, testcase: DataDrivenTestCase) -> None:
)
if "ErrorOutput" in testcase.name or cmd.returncode != 0:
out += cmd.stdout
elif "WarningOutput" in testcase.name:
# Strip out setuptools build related output since we're only
# interested in the messages emitted during compilation.
messages, _, _ = cmd.stdout.partition(b"running build_ext")
out += messages

if cmd.returncode == 0:
# Run main program
Expand Down

0 comments on commit e88f4a4

Please sign in to comment.