diff --git a/guppylang-internals/src/guppylang_internals/cfg/builder.py b/guppylang-internals/src/guppylang_internals/cfg/builder.py index 4ffeb0b7e..d352407eb 100644 --- a/guppylang-internals/src/guppylang_internals/cfg/builder.py +++ b/guppylang-internals/src/guppylang_internals/cfg/builder.py @@ -25,7 +25,7 @@ UnsupportedError, ) from guppylang_internals.checker.errors.type_errors import WrongNumberOfArgsError -from guppylang_internals.diagnostic import Error +from guppylang_internals.diagnostic import Error, Warning from guppylang_internals.error import GuppyError, InternalGuppyError from guppylang_internals.experimental import ( check_lists_enabled, @@ -48,6 +48,7 @@ ) from guppylang_internals.span import Span, to_span from guppylang_internals.tys.ty import NoneType, UnitaryFlags +from guppylang_internals.warning import emit_warning # In order to build expressions, need an endless stream of unique temporary variables # to store intermediate results @@ -68,7 +69,7 @@ class Jumps(NamedTuple): @dataclass(frozen=True) -class UnreachableError(Error): +class UnreachableWarning(Warning): title: ClassVar[str] = "Unreachable" span_label: ClassVar[str] = "This code is not reachable" @@ -135,6 +136,11 @@ def build( raise GuppyError(err) self.cfg.link(final_bb, self.cfg.exit_bb) + for bb in self.cfg.bbs: + if bb.reachable or bb.is_exit or not bb.statements: + continue + emit_warning(UnreachableWarning(bb.statements[0])) + # Prune the CFG such that there are no jumps from unreachable code back into # reachable code. Otherwise, unreachable code could lead to unnecessary type # checking errors, e.g. if unreachable code changes the type of a variable. diff --git a/tests/diagnostics/snapshots/miette/test_unreachable_warning.txt b/tests/diagnostics/snapshots/miette/test_unreachable_warning.txt new file mode 100644 index 000000000..f765a2b7a --- /dev/null +++ b/tests/diagnostics/snapshots/miette/test_unreachable_warning.txt @@ -0,0 +1,7 @@ + ! Unreachable + ,-[3:5] + 2 | return 0 + 3 | x = 1 + : ^^|^^ + : `-- This code is not reachable + `---- \ No newline at end of file diff --git a/tests/diagnostics/snapshots/test_unreachable_warning.txt b/tests/diagnostics/snapshots/test_unreachable_warning.txt new file mode 100644 index 000000000..2da88592d --- /dev/null +++ b/tests/diagnostics/snapshots/test_unreachable_warning.txt @@ -0,0 +1,6 @@ +Warning: Unreachable (at :3:4) + | +1 | def foo(): +2 | return 0 +3 | x = 1 + | ^^^^^ This code is not reachable \ No newline at end of file diff --git a/tests/diagnostics/test_diagnostics_rendering.py b/tests/diagnostics/test_diagnostics_rendering.py index 2ceb40b2d..b2d9ee123 100644 --- a/tests/diagnostics/test_diagnostics_rendering.py +++ b/tests/diagnostics/test_diagnostics_rendering.py @@ -4,6 +4,7 @@ from pathlib import Path from typing import ClassVar +from guppylang_internals.cfg.builder import UnreachableWarning from guppylang_internals.diagnostic import ( Diagnostic, DiagnosticsRenderer, @@ -164,6 +165,13 @@ class MySubDiagnostic(Note): run_test(source, diagnostic, snapshot, request) +def test_unreachable_warning(snapshot, request): + source = "def foo():\n return 0\n x = 1\n" + span = Span(Loc(file, 3, 4), Loc(file, 3, 9)) + diagnostic = UnreachableWarning(span) + run_test(source, diagnostic, snapshot, request) + + def test_context(snapshot, request): source = "super_apple := apple ** 2\nlemon := orange - apple\napple == orange" span = Span(Loc(file, 3, 6), Loc(file, 3, 8)) diff --git a/tests/diagnostics/test_miette_rendering.py b/tests/diagnostics/test_miette_rendering.py index 6a994f059..c4cf1479a 100644 --- a/tests/diagnostics/test_miette_rendering.py +++ b/tests/diagnostics/test_miette_rendering.py @@ -5,6 +5,7 @@ from typing import ClassVar import pytest +from guppylang_internals.cfg.builder import UnreachableWarning from guppylang_internals.diagnostic import ( Diagnostic, DiagnosticLevel, @@ -107,6 +108,15 @@ class WarningDiag(Error): run_miette_test(source, diagnostic, snapshot, request) +def test_unreachable_warning(snapshot, request): + """Test rendering of the concrete unreachable-code warning.""" + + source = "def foo():\n return 0\n x = 1\n" + span = Span(Loc(file, 3, 4), Loc(file, 3, 9)) + diagnostic = UnreachableWarning(span) + run_miette_test(source, diagnostic, snapshot, request) + + def test_complete_issue_example(snapshot, request): """Test complete example from issue #968 with primary + sub-diagnostics.""" diff --git a/tests/error/test_misc_errors.py b/tests/error/test_misc_errors.py index ad663c35f..b258d8a99 100644 --- a/tests/error/test_misc_errors.py +++ b/tests/error/test_misc_errors.py @@ -1,7 +1,8 @@ import pathlib -import pytest +import pytest from guppylang import guppy + from tests.error.util import run_error_test path = pathlib.Path(__file__).parent.resolve() / "misc_errors" diff --git a/tests/error/util.py b/tests/error/util.py index 8cffa5115..d41152dc0 100644 --- a/tests/error/util.py +++ b/tests/error/util.py @@ -37,6 +37,7 @@ def filter_traceback_not_containing(s: str, disallowed_regex: re.Pattern[str]) - return "\n".join(result) + def run_error_test(file, capsys, snapshot): file = pathlib.Path(file) diff --git a/tests/integration/test_unreachable.py b/tests/integration/test_unreachable.py index 6944e7f55..6a05a8b67 100644 --- a/tests/integration/test_unreachable.py +++ b/tests/integration/test_unreachable.py @@ -1,6 +1,27 @@ +import warnings + +import pytest + from guppylang import guppy, qubit +from guppylang import GuppyWarning +from guppylang_internals.error import GuppyError from guppylang.std.quantum import discard, h -from tests.util import compile_guppy +from tests.util import compile_guppy, guppy_warning_records + + +def assert_unreachable_warning_emitted(fn): + """Assert that running `fn` emits exactly one unreachable-code warning.""" + + with warnings.catch_warnings(record=True) as records: + warnings.simplefilter("always") + result = fn() + + guppy_records = guppy_warning_records(records) + assert len(guppy_records) == 1 + warning = guppy_records[0] + assert warning.category is GuppyWarning + assert str(warning.message) == "Unreachable: This code is not reachable" + return result def test_var_defined1(validate): @@ -26,60 +47,75 @@ def test(b: bool) -> int: def test_type_mismatch1(validate): - @compile_guppy - def test() -> int: - if True: - x = 1 - else: - x = 1.0 - return x + def compile_test(): + @compile_guppy + def test() -> int: + if True: + x = 1 + else: + x = 1.0 + return x - validate(test) + return test + + validate(assert_unreachable_warning_emitted(compile_test)) def test_type_mismatch2(validate): - @compile_guppy - def test() -> int: - x = 1 - while False: - x = 1.0 - return x + def compile_test(): + @compile_guppy + def test() -> int: + x = 1 + while False: + x = 1.0 + return x - validate(test) + return test + + validate(assert_unreachable_warning_emitted(compile_test)) def test_type_mismatch3(validate): - @compile_guppy - def test() -> int: - x = 1 - if False and (x := 1.0): - pass - return x + def compile_test(): + @compile_guppy + def test() -> int: + x = 1 + if False and (x := 1.0): + pass + return x - validate(test) + return test + + validate(assert_unreachable_warning_emitted(compile_test)) def test_unused_var_use1(validate): - @compile_guppy - def test() -> int: - x = 1 - if True: - return 0 - return x + def compile_test(): + @compile_guppy + def test() -> int: + x = 1 + if True: + return 0 + return x - validate(test) + return test + + validate(assert_unreachable_warning_emitted(compile_test)) def test_unused_var_use2(validate): - @compile_guppy - def test() -> int: - x = 1 - if not False: - x = 1.0 - return 0 - return x + def compile_test(): + @compile_guppy + def test() -> int: + x = 1 + if not False: + x = 1.0 + return 0 + return x - validate(test) + return test + + validate(assert_unreachable_warning_emitted(compile_test)) def test_unreachable_leak(validate): @@ -93,7 +129,7 @@ def test(b: bool) -> int: # This return would leak, but we don't complain since it's unreachable: return 0 - validate(test.compile_function()) + validate(assert_unreachable_warning_emitted(test.compile_function)) def test_unreachable_leak2(validate): @@ -103,7 +139,7 @@ def test() -> None: # This would leak, but we don't complain since it's unreachable: q = qubit() - validate(test.compile_function()) + validate(assert_unreachable_warning_emitted(test.compile_function)) def test_unreachable_copy(validate): @@ -116,4 +152,61 @@ def test() -> None: # unreachable: h(q) - validate(test.compile_function()) + validate(assert_unreachable_warning_emitted(test.compile_function)) + + +def test_if_false_emits_warning(): + """Statically unreachable branches should emit a single compiler warning.""" + + with warnings.catch_warnings(record=True) as records: + warnings.simplefilter("always") + + @compile_guppy + def test() -> int: + if False: + return 1 + return 0 + + guppy_records = guppy_warning_records(records) + assert len(guppy_records) == 1 + warning = guppy_records[0] + assert warning.category is GuppyWarning + assert warning.filename.endswith("test_unreachable.py") + assert str(warning.message) == "Unreachable: This code is not reachable" + + +def test_dead_code_after_return_emits_warning(): + """Statements after an unconditional return should be reported as unreachable.""" + + with warnings.catch_warnings(record=True) as records: + warnings.simplefilter("always") + + @compile_guppy + def test() -> int: + return 0 + x = 1 + return x + + guppy_records = guppy_warning_records(records) + assert len(guppy_records) == 1 + warning = guppy_records[0] + assert warning.category is GuppyWarning + assert warning.filename.endswith("test_unreachable.py") + assert str(warning.message) == "Unreachable: This code is not reachable" + + +def test_unreachable_warning_is_discarded_if_compilation_fails(): + """Warnings should not leak when unreachable code still contains a hard error.""" + + with warnings.catch_warnings(record=True) as records: + warnings.simplefilter("always") + with pytest.raises(GuppyError): + + @compile_guppy + def test() -> int: + if False: + return 1.0 + return 0 + + guppy_records = guppy_warning_records(records) + assert len(guppy_records) == 0 diff --git a/tests/integration/test_warning_rendering.py b/tests/integration/test_warning_rendering.py new file mode 100644 index 000000000..06b4d74bc --- /dev/null +++ b/tests/integration/test_warning_rendering.py @@ -0,0 +1,28 @@ +import importlib +import pathlib +import warnings + +from tests.util import guppy_warning_records + + +def run_warning_test(file: pathlib.Path, capsys, snapshot) -> None: + """Snapshot rich warning rendering for a module-level compiler warning.""" + with warnings.catch_warnings(record=True) as records: + warnings.simplefilter("always") + importlib.import_module(f"tests.integration.{file.parent.name}.{file.stem}") + + guppy_records = guppy_warning_records(records) + assert len(guppy_records) == 1 + err = capsys.readouterr().err + err = err.replace(str(file), "$FILE") + + snapshot.snapshot_dir = str(file.parent) + snapshot.assert_match(err, file.with_suffix(".err").name) + + +def test_check_warning(capsys, snapshot): + """Rich warnings should snapshot the rendered diagnostic output.""" + file = ( + pathlib.Path(__file__).parent.resolve() / "warning_cases" / "check_warning.py" + ) + run_warning_test(file, capsys, snapshot) diff --git a/tests/integration/warning_cases/__init__.py b/tests/integration/warning_cases/__init__.py new file mode 100644 index 000000000..8b1378917 --- /dev/null +++ b/tests/integration/warning_cases/__init__.py @@ -0,0 +1 @@ + diff --git a/tests/integration/warning_cases/check_warning.err b/tests/integration/warning_cases/check_warning.err new file mode 100644 index 000000000..63550976c --- /dev/null +++ b/tests/integration/warning_cases/check_warning.err @@ -0,0 +1,6 @@ +Warning: Unreachable (at $FILE:8:8) + | +6 | def foo() -> int: +7 | if False: +8 | return 1 + | ^^^^^^^^ This code is not reachable diff --git a/tests/integration/warning_cases/check_warning.py b/tests/integration/warning_cases/check_warning.py new file mode 100644 index 000000000..2bd34863d --- /dev/null +++ b/tests/integration/warning_cases/check_warning.py @@ -0,0 +1,13 @@ +from guppylang import rich_warnings +from guppylang.decorator import guppy + + +@guppy +def foo() -> int: + if False: + return 1 + return 0 + + +with rich_warnings(): + foo.check()