From 8f01b98cc7981deb74687fbb9ff0c96dbff34066 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pierre=20M=C3=A9tras?= Date: Mon, 6 Apr 2026 20:05:13 -0400 Subject: [PATCH 1/6] Broken multi-files package reports illegal name reuses --- src/libponyc/pkg/package.c | 11 ++++++++ test/libponyc/CMakeLists.txt | 3 +++ test/libponyc/badpony.cc | 27 +++++++++++++++++++ .../bad.pony | 4 +++ .../good.pony | 16 +++++++++++ test/libponyc/util.cc | 6 +++++ test/libponyc/util.h | 3 +++ 7 files changed, 70 insertions(+) create mode 100644 test/libponyc/fixtures/spurious-error-message-generation/bad.pony create mode 100644 test/libponyc/fixtures/spurious-error-message-generation/good.pony diff --git a/src/libponyc/pkg/package.c b/src/libponyc/pkg/package.c index 0f68a159e5..1ec5ed5586 100644 --- a/src/libponyc/pkg/package.c +++ b/src/libponyc/pkg/package.c @@ -1052,7 +1052,10 @@ ast_t* package_load(ast_t* from, const char* path, pass_opt_t* opt) return NULL; } else if(magic->mapped_path != NULL) { if(!parse_files_in_dir(package, magic->mapped_path, opt)) + { + ast_setflag(package, AST_FLAG_PRESERVE); return NULL; + } } else { return NULL; } @@ -1060,7 +1063,15 @@ ast_t* package_load(ast_t* from, const char* path, pass_opt_t* opt) else { if(!parse_files_in_dir(package, full_path, opt)) + { + // If source file parsing failed, don't run future passes on the + // partially-initialised package. Without this, passes that run on the + // whole program tree (e.g. the scope pass) would visit modules that + // were successfully parsed in this package but whose for-loop sugar + // has not been applied, producing spurious errors in those files. + ast_setflag(package, AST_FLAG_PRESERVE); return NULL; + } } if(ast_child(package) == NULL) diff --git a/test/libponyc/CMakeLists.txt b/test/libponyc/CMakeLists.txt index ef49aea689..d3bd5ce3e6 100644 --- a/test/libponyc/CMakeLists.txt +++ b/test/libponyc/CMakeLists.txt @@ -67,13 +67,16 @@ target_include_directories(libponyc.tests ) set(PONY_PACKAGES_DIR "${CMAKE_SOURCE_DIR}/packages") +set(PONY_TEST_LIBPONYC_DIR "${CMAKE_SOURCE_DIR}/test/libponyc") if (MSVC) string(REPLACE "/" "\\\\" PONY_PACKAGES_DIR ${PONY_PACKAGES_DIR}) + string(REPLACE "/" "\\\\" PONY_TEST_LIBPONYC_DIR ${PONY_TEST_LIBPONYC_DIR}) endif (MSVC) target_compile_definitions(libponyc.tests PRIVATE PONY_PACKAGES_DIR="${PONY_PACKAGES_DIR}" + PRIVATE PONY_TEST_LIBPONYC_DIR="${PONY_TEST_LIBPONYC_DIR}" ) target_link_directories(libponyc.tests diff --git a/test/libponyc/badpony.cc b/test/libponyc/badpony.cc index 294c8ec022..26eaa97ef7 100644 --- a/test/libponyc/badpony.cc +++ b/test/libponyc/badpony.cc @@ -1446,3 +1446,30 @@ TEST_F(BadPonyTest, MatchArrayPatternWithBareIntegerLiterals) "couldn't find 'eq' in 'Array'", "this pattern element doesn't support structural equality"); } + +TEST_F(BadPonyTest, SpuriousErrorMessageGeneration) +{ + // From issue #4160 + // A parse error in one file of a package must not produce spurious + // "can't reuse name" errors in other files of the same package that + // reuse a loop variable name across consecutive for loops. + // The fixture package has two files: bad.pony (parse error) comes before + // good.pony (valid, with consecutive "for i" loops) alphabetically. + add_package_path("pkg", PONY_TEST_LIBPONYC_DIR "/fixtures/spurious-error-message-generation"); + const char* src = + "use \"pkg\"\n" + "actor Main\n" + " new create(env: Env) =>\n" + " for i in Range(0, 3) do\n" + " None\n" + " end\n" + " for i in Range(0, 3) do\n" + " None\n" + " end"; + + TEST_ERRORS_2(src, + "syntax error: unexpected token = after type, interface, trait, primitive," + " class or actor definition", + "can't load package 'pkg'"); +} + diff --git a/test/libponyc/fixtures/spurious-error-message-generation/bad.pony b/test/libponyc/fixtures/spurious-error-message-generation/bad.pony new file mode 100644 index 0000000000..a88a5c8865 --- /dev/null +++ b/test/libponyc/fixtures/spurious-error-message-generation/bad.pony @@ -0,0 +1,4 @@ +// Intentional syntax error: = instead of => +class Bad + new create() = + None diff --git a/test/libponyc/fixtures/spurious-error-message-generation/good.pony b/test/libponyc/fixtures/spurious-error-message-generation/good.pony new file mode 100644 index 0000000000..92581c2305 --- /dev/null +++ b/test/libponyc/fixtures/spurious-error-message-generation/good.pony @@ -0,0 +1,16 @@ +// Syntactically valid file in the same package as bad.pony. +// The loop variable 'i' is reused across consecutive for loops; before the +// fix for issue #4160, a parse error in bad.pony caused spurious +// "can't reuse name 'i'" errors here. +class Good + new create() => + let a: Array[U8] = Array[U8] + for i in a.values() do + None + end + for i in a.values() do + None + end + for i in a.values() do + None + end diff --git a/test/libponyc/util.cc b/test/libponyc/util.cc index e3c12649ea..91173986b6 100644 --- a/test/libponyc/util.cc +++ b/test/libponyc/util.cc @@ -310,6 +310,12 @@ void PassTest::add_package(const char* path, const char* src) } +void PassTest::add_package_path(const char* path, const char* real_dir) +{ + package_add_magic_path(path, real_dir, &opt); +} + + void PassTest::default_package_name(const char* path) { _first_pkg_path = path; diff --git a/test/libponyc/util.h b/test/libponyc/util.h index 986f93c1c3..109fadda79 100644 --- a/test/libponyc/util.h +++ b/test/libponyc/util.h @@ -49,6 +49,9 @@ class PassTest : public testing::Test // Add an additional package source void add_package(const char* path, const char* src); + // Add an additional package backed by a real filesystem directory + void add_package_path(const char* path, const char* real_dir); + // Override the default path of the main package (useful for package loops) void default_package_name(const char* path); From 0d52bf0bbd6366ecb661fb2d38f3a9294c420c7c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pierre=20M=C3=A9tras?= Date: Mon, 6 Apr 2026 22:16:22 -0400 Subject: [PATCH 2/6] Add release notes --- .release-notes/5147.md | 22 ++++++++++++++++++++++ src/libponyc/pkg/package.c | 8 ++++---- 2 files changed, 26 insertions(+), 4 deletions(-) create mode 100644 .release-notes/5147.md diff --git a/.release-notes/5147.md b/.release-notes/5147.md new file mode 100644 index 0000000000..e6778cec14 --- /dev/null +++ b/.release-notes/5147.md @@ -0,0 +1,22 @@ +## Fix spurious 'can't reuse name' errors when a multi-file package has a syntax error + +Fix [issue 4160](https://github.com/ponylang/ponyc/issues/4160) that contains +and example reproducing the problem. +When a package `use`d another package containing multiple `.pony` files, and +one of these files had a syntax error, the compiler output wrong and extra +error messages, particularly when the first code had multiple `for` loops +using the same loop variable. + +The issue was caused by the compiler's package loading logic. When a package +containing multiple files was loaded, if one file had a syntax error, the +package was left in a partially-processed state. Subsequent attempts to use +or "catch up" that package to the current compiler pass would re-run passes +(like the scope pass) on the existing AST. Since the scope pass had already +partially populated the symbol tables, re-running it caused it to encounter +its own previously defined symbols and report them as illegal name reuses. + +The fix sets `AST_FLAG_PRESERVE` on the package when `parse_files_in_dir` fails. +This stops future passes from visiting the broken package. + +The compiler tests were adapted to support testing with a package containing +multiple source files. diff --git a/src/libponyc/pkg/package.c b/src/libponyc/pkg/package.c index 1ec5ed5586..bdff14f9eb 100644 --- a/src/libponyc/pkg/package.c +++ b/src/libponyc/pkg/package.c @@ -1053,6 +1053,8 @@ ast_t* package_load(ast_t* from, const char* path, pass_opt_t* opt) } else if(magic->mapped_path != NULL) { if(!parse_files_in_dir(package, magic->mapped_path, opt)) { + // If source file parsing failed, don't run future passes on the + // partially-initialised package. ast_setflag(package, AST_FLAG_PRESERVE); return NULL; } @@ -1065,10 +1067,8 @@ ast_t* package_load(ast_t* from, const char* path, pass_opt_t* opt) if(!parse_files_in_dir(package, full_path, opt)) { // If source file parsing failed, don't run future passes on the - // partially-initialised package. Without this, passes that run on the - // whole program tree (e.g. the scope pass) would visit modules that - // were successfully parsed in this package but whose for-loop sugar - // has not been applied, producing spurious errors in those files. + // partially-initialised package, producing spurious errors in + // those files. ast_setflag(package, AST_FLAG_PRESERVE); return NULL; } From 59933c01e17479472d68b8744b1bc76599ce5af2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pierre=20M=C3=A9tras?= Date: Tue, 7 Apr 2026 19:29:19 -0400 Subject: [PATCH 3/6] Better release message --- .release-notes/5147.md | 26 ++++++++------------------ 1 file changed, 8 insertions(+), 18 deletions(-) diff --git a/.release-notes/5147.md b/.release-notes/5147.md index e6778cec14..646b1d8672 100644 --- a/.release-notes/5147.md +++ b/.release-notes/5147.md @@ -1,22 +1,12 @@ ## Fix spurious 'can't reuse name' errors when a multi-file package has a syntax error -Fix [issue 4160](https://github.com/ponylang/ponyc/issues/4160) that contains -and example reproducing the problem. -When a package `use`d another package containing multiple `.pony` files, and -one of these files had a syntax error, the compiler output wrong and extra -error messages, particularly when the first code had multiple `for` loops -using the same loop variable. +When a package contained multiple `.pony` files and one of them had a syntax +error, the compiler would produce spurious `can't reuse name` errors for +unrelated valid code in the other files. This was most visible when a valid +file reused a loop variable name across consecutive `for` loops — a perfectly +legal pattern that the compiler would incorrectly flag. -The issue was caused by the compiler's package loading logic. When a package -containing multiple files was loaded, if one file had a syntax error, the -package was left in a partially-processed state. Subsequent attempts to use -or "catch up" that package to the current compiler pass would re-run passes -(like the scope pass) on the existing AST. Since the scope pass had already -partially populated the symbol tables, re-running it caused it to encounter -its own previously defined symbols and report them as illegal name reuses. +The spurious errors no longer appear. You'll still see the real syntax error, +but the compiler won't pile on with bogus errors from other files in the same +package. -The fix sets `AST_FLAG_PRESERVE` on the package when `parse_files_in_dir` fails. -This stops future passes from visiting the broken package. - -The compiler tests were adapted to support testing with a package containing -multiple source files. From 29bbb382935f3c8d6515ba517118f4289f675b11 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pierre=20M=C3=A9tras?= Date: Tue, 7 Apr 2026 19:45:43 -0400 Subject: [PATCH 4/6] Removed lines wrap --- .release-notes/5147.md | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/.release-notes/5147.md b/.release-notes/5147.md index 646b1d8672..23ea5c2df1 100644 --- a/.release-notes/5147.md +++ b/.release-notes/5147.md @@ -1,12 +1,6 @@ ## Fix spurious 'can't reuse name' errors when a multi-file package has a syntax error -When a package contained multiple `.pony` files and one of them had a syntax -error, the compiler would produce spurious `can't reuse name` errors for -unrelated valid code in the other files. This was most visible when a valid -file reused a loop variable name across consecutive `for` loops — a perfectly -legal pattern that the compiler would incorrectly flag. +When a package contained multiple `.pony` files and one of them had a syntax error, the compiler would produce spurious `can't reuse name` errors for unrelated valid code in the other files. This was most visible when a valid file reused a loop variable name across consecutive `for` loops — a perfectly legal pattern that the compiler would incorrectly flag. -The spurious errors no longer appear. You'll still see the real syntax error, -but the compiler won't pile on with bogus errors from other files in the same -package. +The spurious errors no longer appear. You'll still see the real syntax error, but the compiler won't pile on with bogus errors from other files in the same package. From f731f0e9d7df1a8c8b59329c1096067c24809669 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pierre=20M=C3=A9tras?= Date: Mon, 6 Apr 2026 20:05:13 -0400 Subject: [PATCH 5/6] Broken multi-files package reports illegal name reuses --- src/libponyc/pkg/package.c | 8 +-- test/libponyc/badpony.cc | 106 ------------------------------------- 2 files changed, 4 insertions(+), 110 deletions(-) diff --git a/src/libponyc/pkg/package.c b/src/libponyc/pkg/package.c index bdff14f9eb..1ec5ed5586 100644 --- a/src/libponyc/pkg/package.c +++ b/src/libponyc/pkg/package.c @@ -1053,8 +1053,6 @@ ast_t* package_load(ast_t* from, const char* path, pass_opt_t* opt) } else if(magic->mapped_path != NULL) { if(!parse_files_in_dir(package, magic->mapped_path, opt)) { - // If source file parsing failed, don't run future passes on the - // partially-initialised package. ast_setflag(package, AST_FLAG_PRESERVE); return NULL; } @@ -1067,8 +1065,10 @@ ast_t* package_load(ast_t* from, const char* path, pass_opt_t* opt) if(!parse_files_in_dir(package, full_path, opt)) { // If source file parsing failed, don't run future passes on the - // partially-initialised package, producing spurious errors in - // those files. + // partially-initialised package. Without this, passes that run on the + // whole program tree (e.g. the scope pass) would visit modules that + // were successfully parsed in this package but whose for-loop sugar + // has not been applied, producing spurious errors in those files. ast_setflag(package, AST_FLAG_PRESERVE); return NULL; } diff --git a/test/libponyc/badpony.cc b/test/libponyc/badpony.cc index 4bd15d0125..d2df672d65 100644 --- a/test/libponyc/badpony.cc +++ b/test/libponyc/badpony.cc @@ -1762,109 +1762,3 @@ TEST_F(BadPonyTest, SpuriousErrorMessageGeneration) "can't load package 'pkg'"); } -TEST_F(BadPonyTest, IfLiteralBranchWithTypecheckError) -{ - // Exercises the error path in expr_if where the first branch - // produces a literal type (unparented TK_LITERAL accumulator) - // and the second branch has a type error (#5214). - const char* src = - "primitive Foo\n" - " fun bar(x: U32): U32 => x\n" - - "actor Main\n" - " new create(env: Env) =>\n" - " if true then 1 else Foo.bar(true) end"; - - TEST_ERRORS_1(src, "argument not assignable to parameter"); -} - -TEST_F(BadPonyTest, IftypeLiteralBranchWithTypecheckError) -{ - // Same pattern for iftype (#5214). - const char* src = - "trait T\n" - "class C is T\n" - - "primitive Foo\n" - " fun bar(x: U32): U32 => x\n" - - "actor Main\n" - " new create(env: Env) =>\n" - " _test[C]()\n" - - " fun _test[A: T]() =>\n" - " iftype A <: C then 1\n" - " else Foo.bar(true) end"; - - TEST_ERRORS_1(src, - "argument not assignable to parameter"); -} - -TEST_F(BadPonyTest, TryLiteralBodyWithTypecheckErrorElse) -{ - // Same pattern for try (#5214). - const char* src = - "primitive Foo\n" - " fun bar(x: U32): U32 => x\n" - - "actor Main\n" - " new create(env: Env) =>\n" - " try 1 else Foo.bar(true) end"; - - TEST_ERRORS_1(src, "argument not assignable to parameter"); -} - -TEST_F(BadPonyTest, WhileLiteralBodyWithTypecheckErrorElse) -{ - // Same pattern for while: the body produces a literal type, - // then the else clause has a type error (#5214). - const char* src = - "primitive Foo\n" - " fun bar(x: U32): U32 => x\n" - - "actor Main\n" - " new create(env: Env) =>\n" - " while true do 1\n" - " else Foo.bar(true) end"; - - TEST_ERRORS_1(src, "argument not assignable to parameter"); -} - -TEST_F(BadPonyTest, RepeatLiteralBodyWithTypecheckErrorElse) -{ - // Same pattern for repeat (#5214). - const char* src = - "primitive Foo\n" - " fun bar(x: U32): U32 => x\n" - - "actor Main\n" - " new create(env: Env) =>\n" - " repeat 1 until true\n" - " else Foo.bar(true) end"; - - TEST_ERRORS_1(src, "argument not assignable to parameter"); -} - -TEST_F(BadPonyTest, MatchLiteralCaseWithTypecheckErrorElse) -{ - // Same pattern for match: case body is a literal, else clause - // has a type error. Non-exhaustive match so the else clause - // is reached during type checking (#5214). - const char* src = - "primitive Foo\n" - " fun bar(x: U32): U32 => x\n" - - "class Bar\n" - "class Baz\n" - - "actor Main\n" - " new create(env: Env) =>\n" - " let b: (Bar | Baz) = Bar\n" - " match b\n" - " | let _: Bar => 1\n" - " else\n" - " Foo.bar(true)\n" - " end"; - - TEST_ERRORS_1(src, "argument not assignable to parameter"); -} From 0ea00f2ae88b8ce20185e2d16bfac09a0908c208 Mon Sep 17 00:00:00 2001 From: Sean T Allen Date: Sat, 11 Apr 2026 07:57:20 -0400 Subject: [PATCH 6/6] Close error-path leaks of unparented TK_LITERAL nodes (#5219) When control_type_add_branch creates a TK_LITERAL wrapper for a literal-typed branch, the result is stored in a local accumulator variable but never parented until ast_settype at the end of the function. If a subsequent branch hits is_typecheck_error or another error check, the early return leaks that unparented node. Add ast_free_unattached(type) before each early return that can fire after the accumulator has been set. Safe because ast_free_unattached is a no-op on NULL and on parented nodes. Fixed sites: - expr_if (control.c): second-branch typecheck error - expr_iftype (control.c): second-branch typecheck error - expr_while (control.c): else-clause typecheck error - expr_repeat (control.c): else-clause typecheck error - expr_try (control.c): else-clause typecheck error, then-clause typecheck error, then-clause literal type error - expr_disposing_block (control.c): dispose-clause typecheck error, dispose-clause literal type error - expr_match (match.c): exhaustiveness errors, else-clause typecheck error Closes #5214 --- test/libponyc/badpony.cc | 107 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 107 insertions(+) diff --git a/test/libponyc/badpony.cc b/test/libponyc/badpony.cc index d2df672d65..01a431df5f 100644 --- a/test/libponyc/badpony.cc +++ b/test/libponyc/badpony.cc @@ -1762,3 +1762,110 @@ TEST_F(BadPonyTest, SpuriousErrorMessageGeneration) "can't load package 'pkg'"); } + +TEST_F(BadPonyTest, IfLiteralBranchWithTypecheckError) +{ + // Exercises the error path in expr_if where the first branch + // produces a literal type (unparented TK_LITERAL accumulator) + // and the second branch has a type error (#5214). + const char* src = + "primitive Foo\n" + " fun bar(x: U32): U32 => x\n" + + "actor Main\n" + " new create(env: Env) =>\n" + " if true then 1 else Foo.bar(true) end"; + + TEST_ERRORS_1(src, "argument not assignable to parameter"); +} + +TEST_F(BadPonyTest, IftypeLiteralBranchWithTypecheckError) +{ + // Same pattern for iftype (#5214). + const char* src = + "trait T\n" + "class C is T\n" + + "primitive Foo\n" + " fun bar(x: U32): U32 => x\n" + + "actor Main\n" + " new create(env: Env) =>\n" + " _test[C]()\n" + + " fun _test[A: T]() =>\n" + " iftype A <: C then 1\n" + " else Foo.bar(true) end"; + + TEST_ERRORS_1(src, + "argument not assignable to parameter"); +} + +TEST_F(BadPonyTest, TryLiteralBodyWithTypecheckErrorElse) +{ + // Same pattern for try (#5214). + const char* src = + "primitive Foo\n" + " fun bar(x: U32): U32 => x\n" + + "actor Main\n" + " new create(env: Env) =>\n" + " try 1 else Foo.bar(true) end"; + + TEST_ERRORS_1(src, "argument not assignable to parameter"); +} + +TEST_F(BadPonyTest, WhileLiteralBodyWithTypecheckErrorElse) +{ + // Same pattern for while: the body produces a literal type, + // then the else clause has a type error (#5214). + const char* src = + "primitive Foo\n" + " fun bar(x: U32): U32 => x\n" + + "actor Main\n" + " new create(env: Env) =>\n" + " while true do 1\n" + " else Foo.bar(true) end"; + + TEST_ERRORS_1(src, "argument not assignable to parameter"); +} + +TEST_F(BadPonyTest, RepeatLiteralBodyWithTypecheckErrorElse) +{ + // Same pattern for repeat (#5214). + const char* src = + "primitive Foo\n" + " fun bar(x: U32): U32 => x\n" + + "actor Main\n" + " new create(env: Env) =>\n" + " repeat 1 until true\n" + " else Foo.bar(true) end"; + + TEST_ERRORS_1(src, "argument not assignable to parameter"); +} + +TEST_F(BadPonyTest, MatchLiteralCaseWithTypecheckErrorElse) +{ + // Same pattern for match: case body is a literal, else clause + // has a type error. Non-exhaustive match so the else clause + // is reached during type checking (#5214). + const char* src = + "primitive Foo\n" + " fun bar(x: U32): U32 => x\n" + + "class Bar\n" + "class Baz\n" + + "actor Main\n" + " new create(env: Env) =>\n" + " let b: (Bar | Baz) = Bar\n" + " match b\n" + " | let _: Bar => 1\n" + " else\n" + " Foo.bar(true)\n" + " end"; + + TEST_ERRORS_1(src, "argument not assignable to parameter"); +}