diff --git a/.release-notes/5147.md b/.release-notes/5147.md new file mode 100644 index 0000000000..23ea5c2df1 --- /dev/null +++ b/.release-notes/5147.md @@ -0,0 +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. + +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. + diff --git a/src/libponyc/pkg/package.c b/src/libponyc/pkg/package.c index 0f68a159e5..bdff14f9eb 100644 --- a/src/libponyc/pkg/package.c +++ b/src/libponyc/pkg/package.c @@ -1052,7 +1052,12 @@ 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)) + { + // If source file parsing failed, don't run future passes on the + // partially-initialised package. + ast_setflag(package, AST_FLAG_PRESERVE); return NULL; + } } else { return NULL; } @@ -1060,7 +1065,13 @@ 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, 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 c40cb65fee..e3dd428978 100644 --- a/test/libponyc/CMakeLists.txt +++ b/test/libponyc/CMakeLists.txt @@ -68,13 +68,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 f1f5b71b14..d2df672d65 100644 --- a/test/libponyc/badpony.cc +++ b/test/libponyc/badpony.cc @@ -1735,3 +1735,30 @@ TEST_F(BadPonyTest, MatchTuplePatternFromAliasedTupleUnsound) "this capture is unsound", "this capture is unsound"); } + +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);