diff --git a/crates/compiler/src/compile.rs b/crates/compiler/src/compile.rs index d6c3957..fffb7b7 100644 --- a/crates/compiler/src/compile.rs +++ b/crates/compiler/src/compile.rs @@ -1694,6 +1694,7 @@ impl<'a> VarIdTyPass<'a> { fn execute(&mut self) -> AnnotatedAst { let mut decls = Vec::new(); + self.check_duplicate_decls(); // Enum types must exist before imports and function signatures are // resolved. Imports are then installed before structs and functions // are declared, allowing imported enum and struct types in fields and @@ -1767,6 +1768,44 @@ impl<'a> VarIdTyPass<'a> { ) } + /// Reports every top-level declaration whose name an earlier declaration + /// of this module already took. + /// + /// Cells, functions, structs, enums, and imports all bind into the one + /// module frame, so `struct Mode` after `enum Mode` clashes as much as two + /// `cell top`s do. The declaration passes run by kind rather than in + /// source order, so without this check the survivor of a clash was + /// whichever kind is declared last -- a struct always beat an enum of the + /// same name, whatever the file said -- and [`Self::declare_struct_decls`] + /// keys structs by name, so a repeated `struct S` dropped the first + /// declaration without a trace. The later declaration is reported and + /// binding proceeds as before; the file is already invalid. + /// + /// Modules are exempt: `mod m;` is resolved through `mod_bindings`, never + /// through this frame, so `mod m;` and `fn m` do not collide. + fn check_duplicate_decls(&mut self) { + let mut seen = IndexSet::new(); + for decl in &self.ast.ast.decls { + let name = match decl { + Decl::Enum(e) => &e.name, + Decl::Struct(s) => &s.name, + Decl::Fn(f) => &f.name, + Decl::Cell(c) => &c.name, + Decl::Use(u) => u + .alias + .as_ref() + .unwrap_or_else(|| u.path.last().expect("use paths are non-empty")), + Decl::Mod(_) | Decl::Constant(_) => continue, + }; + if !seen.insert(name.name.as_str()) { + self.errors.push(StaticError { + span: self.span(name.span), + kind: StaticErrorKind::DuplicateNameDeclaration, + }); + } + } + } + fn declare_use_decl(&mut self, use_decl: &UseDecl) { let module = self.use_module_path(use_decl); let item = use_decl.path.last().expect("use paths are non-empty"); diff --git a/crates/compiler/src/compile/result.rs b/crates/compiler/src/compile/result.rs index 7fca422..5e69722 100644 --- a/crates/compiler/src/compile/result.rs +++ b/crates/compiler/src/compile/result.rs @@ -16,7 +16,10 @@ pub struct StaticError { pub enum StaticErrorKind { /// Multiple declarations with the same name. /// - /// For example, two cells named `my_cell`. + /// For example, two cells named `my_cell`. Top-level cells, functions, + /// structs, enums, and imports share one namespace per module, so + /// `struct Mode` after `enum Mode` is reported too. Also covers repeated + /// parameter names, enum variants, and struct fields. #[error("duplicate name declaration")] DuplicateNameDeclaration, /// Attempted to declare an object with the same name as a built-in object. diff --git a/crates/compiler/src/lib.rs b/crates/compiler/src/lib.rs index f5b7ea6..4132b31 100644 --- a/crates/compiler/src/lib.rs +++ b/crates/compiler/src/lib.rs @@ -1961,6 +1961,75 @@ mod tests { ); } + #[test] + fn top_level_declarations_share_one_namespace() { + // `bind` overwrote without checking, so `struct Mode` after + // `enum Mode` quietly won, `struct F` and `fn F` resolved to whichever + // kind the declaration passes visit last, and two `struct S` kept only + // the second. + let duplicates = |source: &str| { + static_errors(source) + .into_iter() + .filter(|error| matches!(error.kind, StaticErrorKind::DuplicateNameDeclaration)) + .collect::>() + }; + for source in [ + "enum Mode { A, B }\nstruct Mode { a: Int, }\n", + "struct Mode { a: Int, }\nenum Mode { A, B }\n", + "struct F { a: Int, }\nfn F() -> Int { 1 }\n", + "fn F() -> Int { 1 }\nstruct F { a: Int, }\n", + "struct S { a: Int, }\nstruct S { b: Float, }\n", + "enum E { A }\nenum E { B }\n", + "fn f() -> Int { 1 }\nfn f() -> Float { 1. }\n", + "cell top() {}\ncell top() {}\n", + "fn top() -> Int { 1 }\ncell top() {}\n", + "cell top() {}\nenum top { A }\n", + // An import takes a name like any declaration does. + "use std::max;\nfn max(a: Float) -> Float { a }\n", + "use std::max;\nuse std::min as max;\n", + ] { + let errors = duplicates(source); + assert_eq!(errors.len(), 1, "{source:?}: {errors:?}"); + } + + // The later declaration is the one reported, at its name. + let source = "enum Mode { A, B }\nstruct Mode { a: Int, }\n"; + let errors = duplicates(source); + let [error] = errors.as_slice() else { + unreachable!("checked above") + }; + let name = source.rfind("Mode").unwrap(); + assert_eq!(error.span.span.start(), name); + assert_eq!(error.span.span.end(), name + "Mode".len()); + + // Every repeat is reported, not just the first. + assert_eq!( + duplicates("cell a() {}\ncell a() {}\ncell a() {}\n").len(), + 2 + ); + + // Distinct names in any mix of kinds are fine, and a name may be + // reused across modules: `std` declares `max` too. + assert!( + duplicates( + "enum A { X }\nstruct B { a: A, }\nfn c(b: B) -> A { b.a }\ncell d() {}\n\ + fn max(a: Float) -> Float { a }\n" + ) + .is_empty() + ); + + // Modules live in their own namespace, so `mod m;` and `fn m` coexist. + let root = parse_source_text( + "mod m;\nfn m() -> Int { m::h() }\n", + PathBuf::from("/virtual/lib.ar"), + ) + .unwrap(); + let m = parse_source_text("fn h() -> Int { 2 }\n", PathBuf::from("/virtual/m.ar")).unwrap(); + let ast = IndexMap::from([(Vec::new(), root), (vec!["m".to_owned()], m)]); + let (_, output) = static_compile(&ast).unwrap(); + assert!(output.errors.is_empty(), "{:?}", output.errors); + } + #[test] fn argon_library() { let o = parse_workspace_with_std(ARGON_LIBRARY);