From 59f7661aa781e185316031d6e36f583b8e9b663a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Joaqu=C3=ADn=20Gamboa?= Date: Mon, 19 Oct 2026 12:51:33 -0300 Subject: [PATCH 1/6] fix(checker): file-qualified diagnostics, autoload span identities and preludes, checker cost Diagnostics: - Each autoloaded file gets its own source id, and the resolver of its includes no longer rewinds the counter, so spans of different files no longer collide (two autoloaded files with a retype at the same line and column no longer fail with "Cannot re-bind $x here"). - Span::register_source_path / source_path map ids to files; print_error and print_warning use it when the diagnostic carries no file, so every positioned diagnostic is now `kind[file:line:col]`, entry file included. - Class schema errors (circular inheritance, final class, duplicate class, unknown parent) point at the class declaration instead of Span::dummy(). Preludes: autoload::prelude_probe resolves names and runs the autoload on a copy of the program, so pdo, mysqli, tz, image, hash, curl, xml and var_export are injected when only an autoloaded file uses them. The probe rewinds every counter it advanced. Method variadics: widen_method_variadic_element only widens, so a collector a first-class callable promoted to array is not narrowed back to array by a direct string call (the callable path then stored mixed values into a string collector: "heap memory exhausted"). Cost (3400-class program, 290 s -> 62 s user time, same result): indexes for method-implementation users, property sharers, loop-storage scopes and class descendants (new static), and exact-key lookups before the case-insensitive scans in check_builtin, has_function_decl_folded and folded_map_key. --- README.md | 16 +- src/autoload/mod.rs | 12 +- src/autoload/prelude_probe.rs | 75 ++++++++ src/curl_prelude.rs | 8 + src/errors/report.rs | 41 ++++- src/hash_prelude.rs | 8 + src/image_prelude.rs | 8 + src/parser/mod.rs | 11 ++ src/pipeline.rs | 44 ++++- src/resolver/engine_includes.rs | 2 +- src/resolver/files.rs | 1 + src/resolver/mod.rs | 37 +++- src/span.rs | 51 ++++++ src/types/checker/builtins/mod.rs | 6 +- src/types/checker/driver/functions.rs | 11 ++ src/types/checker/driver/init.rs | 4 + src/types/checker/driver/mod.rs | 53 +++++- .../checker/functions/resolution/signature.rs | 3 +- .../checker/inference/expr/class_refs.rs | 25 +-- .../checker/inference/objects/methods.rs | 68 ++++++- src/types/checker/inference/ops.rs | 3 +- src/types/checker/method_pass.rs | 74 ++++++-- src/types/checker/mod.rs | 23 +++ src/types/checker/schema/classes/mod.rs | 50 +++++- src/types/checker/stmt_check/control_flow.rs | 21 ++- src/types/checker/type_compat/object_types.rs | 39 +++- src/tz_prelude.rs | 8 + src/var_export_prelude.rs | 22 ++- src/xml_prelude.rs | 8 + tests/autoload_diagnostics_tests.rs | 170 ++++++++++++++++++ .../codegen/callables/state_and_variadics.rs | 27 +++ 31 files changed, 842 insertions(+), 87 deletions(-) create mode 100644 src/autoload/prelude_probe.rs create mode 100644 tests/autoload_diagnostics_tests.rs diff --git a/README.md b/README.md index 748e74e965..a0b7174fc4 100644 --- a/README.md +++ b/README.md @@ -651,17 +651,19 @@ A variable's type is set at first assignment. Compatible types (int/float/bool/n ## Error messages -Errors include line and column numbers, and the compiler tries to recover far enough to report multiple independent syntax / semantic errors in one pass. Successful compilations may also emit non-fatal warnings such as unused variables / parameters or unreachable code: +Errors name the file, line and column they come from (the entry file as given on the command line, or the included or autoloaded file that holds the statement), and the compiler tries to recover far enough to report multiple independent syntax / semantic errors in one pass. Successful compilations may also emit non-fatal warnings such as unused variables / parameters or unreachable code: ``` -error[3:1]: Undefined variable: $x -error[5:7]: Type error: cannot reassign $x from int to string -error[2:1]: Required file not found: 'missing.php' -warning[6:1]: $a changes type from int to string; the previous value is discarded (compile with --strict-locals to make this an error) -warning[9:5]: Unused variable: $tmp -warning[14:9]: Unreachable code +error[main.php:3:1]: Undefined variable: $x +error[main.php:5:7]: Type error: cannot reassign $x from int to string +error[main.php:2:1]: Required file not found: 'missing.php' +warning[main.php:6:1]: $a changes type from int to string; the previous value is discarded (compile with --strict-locals to make this an error) +warning[main.php:9:5]: Unused variable: $tmp +warning[src/Helpers.php:14:9]: Unreachable code ``` +A diagnostic with no source position (some whole-program checks) prints `error: message`. + ## Project structure High-level map of the source tree. The codebase contains more focused helper submodules than shown here; treat this as an orientation guide rather than a byte-for-byte file listing. diff --git a/src/autoload/mod.rs b/src/autoload/mod.rs index fc596cab31..d096c533a2 100644 --- a/src/autoload/mod.rs +++ b/src/autoload/mod.rs @@ -13,6 +13,7 @@ mod alias; mod dev_packages; +pub mod prelude_probe; mod index; mod interpret; mod registry; @@ -212,13 +213,18 @@ fn load_autoloaded_file( })?; let file_label = path.display().to_string(); let source_mode = crate::source::SourceMode::from_path(path); - let tokens = crate::lexer::tokenize_with_mode(&content, source_mode) - .map_err(|e| e.with_file(file_label.clone()))?; + // Its own source identity, so its spans differ from every other file's and a diagnostic + // raised anywhere in it can name it (`Span::source_path`). + let source_id = Span::fresh_source_id(); + Span::register_source_path(source_id, file_label.clone()); + let tokens = + crate::lexer::tokenize_with_mode_and_source_id(&content, source_mode, source_id) + .map_err(|e| e.with_file(file_label.clone()))?; let parsed = crate::parser::parse_with_mode(&tokens, source_mode) .map_err(|e| e.with_file(file_label.clone()))?; let parsed = crate::source::finalize_physical_program(parsed, &content, path, source_mode, defines)?; - let (resolved, nested_includes) = crate::resolver::resolve_collecting_includes_with_defines( + let (resolved, nested_includes) = crate::resolver::resolve_collecting_includes_continuing( parsed, path.parent().unwrap_or(base_dir), defines, diff --git a/src/autoload/prelude_probe.rs b/src/autoload/prelude_probe.rs new file mode 100644 index 0000000000..ab48ae0474 --- /dev/null +++ b/src/autoload/prelude_probe.rs @@ -0,0 +1,75 @@ +//! Purpose: +//! Decides which detected standard-library preludes the AUTOLOAD CLOSURE of a program needs, +//! before those preludes are injected. +//! +//! Called from: +//! - `crate::pipeline::compile()`, right before the prelude injections. +//! +//! Key details: +//! - Each prelude (PDO, mysqli, tz, image, hash, curl, xml, var_export) is injected only when +//! the program uses its surface, and that decision has to be taken before name resolution so a +//! namespaced caller resolves to the injected declarations. The Composer/SPL autoload pass runs +//! AFTER name resolution, so a class file it loads that uses `PDO` or `CurlHandle` was never +//! scanned: the prelude was skipped and the program failed with `Unknown type: PDO`. +//! - The probe runs name resolution and the autoload pass on a COPY of the program, scans the +//! result with each prelude's own detector, and hands back the groups found. The real pass then +//! injects them exactly as if the entry file had used them, so the outcome is the one +//! `--with-` gives, without forcing the whole surface past reachability pruning. +//! - The copy's work is discarded, including every counter it advanced (source identities, +//! synthetic spans, anonymous-class numbers, the resolver's hidden temporaries), so the real +//! pass numbers its files, classes and temporaries exactly as it would without the probe. +//! - A probe failure is not reported here: the real pass hits the same error and reports it. + +use std::collections::HashSet; +use std::path::Path; + +use crate::parser::ast::{Program, Stmt}; +use crate::span::Span; + +use super::Registry; + +/// Every prelude group the probe can report, with the detector that decides it. +const PROBED_GROUPS: &[(&str, fn(&[Stmt]) -> bool)] = &[ + ("pdo", crate::pdo_prelude::program_uses_pdo), + ("mysqli", crate::mysqli_prelude::program_uses_mysqli), + ("tz", crate::tz_prelude::program_uses_surface), + ("image", crate::image_prelude::program_uses_surface), + ("hash", crate::hash_prelude::program_uses_surface), + ("curl", crate::curl_prelude::program_uses_surface), + ("xml", crate::xml_prelude::program_uses_surface), + ("var_export", crate::var_export_prelude::program_uses_surface), +]; + +/// Returns the prelude groups some file of the autoload closure uses. +/// +/// `program` is the include-resolved entry program, before any prelude is injected. Returns an +/// empty set when nothing is autoloadable or the probe cannot finish. +pub fn autoloaded_prelude_groups( + program: &Program, + base_dir: &Path, + registry: &Registry, + defines: &HashSet, +) -> HashSet<&'static str> { + if registry.is_empty() { + return HashSet::new(); + } + let span_counters = Span::counters_checkpoint(); + let resolver_counters = crate::resolver::counters_checkpoint(); + let anonymous_classes = crate::parser::anonymous_class_counter(); + let closure = crate::name_resolver::resolve(program.clone()) + .ok() + .and_then(|resolved| { + super::run_collecting_included_with_defines(resolved, base_dir, registry, defines).ok() + }); + Span::restore_counters(span_counters); + crate::resolver::restore_counters(resolver_counters); + crate::parser::restore_anonymous_class_counter(anonymous_classes); + let Some((closure, _)) = closure else { + return HashSet::new(); + }; + PROBED_GROUPS + .iter() + .filter(|(_, detect)| detect(&closure)) + .map(|(group, _)| *group) + .collect() +} diff --git a/src/curl_prelude.rs b/src/curl_prelude.rs index 865f17b4d6..8b7d666946 100644 --- a/src/curl_prelude.rs +++ b/src/curl_prelude.rs @@ -1924,6 +1924,14 @@ function curl_share_init_persistent(array $share_options): CurlSharePersistentHa // -- elephc PHP >= 8.5 curl_share_init_persistent end -- "#; +/// Whether `program` uses this prelude's surface: the same test `inject_if_used` applies. +/// +/// Also run by `crate::autoload::prelude_probe` over the autoload closure, whose files are only +/// loaded after this prelude has been decided. +pub(crate) fn program_uses_surface(program: &[crate::parser::ast::Stmt]) -> bool { + detect::program_uses_curl(program) +} + /// Injects the curl prelude when the program references the `ext/curl` surface, leaving /// every other program untouched, for the DEFAULT PHP compatibility version. /// diff --git a/src/errors/report.rs b/src/errors/report.rs index 8a9c6a87a4..d8792cb0a1 100644 --- a/src/errors/report.rs +++ b/src/errors/report.rs @@ -18,12 +18,17 @@ use super::{CompileError, CompileWarning}; /// Format varies based on available metadata: /// - With file, line, and column: `error[file:line:col]: message` /// - With file only: `error[file]: message` -/// - With line and column only: `error[line:col]: message` +/// - With line and column only: `error[line:col]: message` (a span whose source file was never +/// registered, such as a synthetic node) /// - With no location metadata: `error: message` /// +/// When the error carries no file of its own, the file its span was lexed from is used +/// (`Span::source_path`), so checker and later diagnostics are file-qualified too. +/// /// Recursively prints any related errors attached to the main error. pub fn print_error(error: &CompileError) { - let plain = format_diagnostic("error", &error.file, error.span, &error.message); + let file = diagnostic_file(&error.file, error.span); + let plain = format_diagnostic("error", &file, error.span, &error.message); if crate::progress::is_decorated() { eprintln!( "{} {}", @@ -41,10 +46,12 @@ pub fn print_error(error: &CompileError) { /// Prints a compiler warning to stderr with optional line and column. /// /// Format varies based on available metadata: -/// - With line and column: `warning[line:col]: message` +/// - With a registered source file, line and column: `warning[file:line:col]: message` +/// - With line and column only: `warning[line:col]: message` /// - With no location metadata: `warning: message` pub fn print_warning(warning: &CompileWarning) { - let plain = format_diagnostic("warning", &None, warning.span, &warning.message); + let file = diagnostic_file(&None, warning.span); + let plain = format_diagnostic("warning", &file, warning.span, &warning.message); if crate::progress::is_decorated() { eprintln!( "{} {}", @@ -56,6 +63,19 @@ pub fn print_warning(warning: &CompileWarning) { } } +/// The file a diagnostic names: the one the error was tagged with, else the file its span was +/// lexed from (`Span::source_path`). Checker and later diagnostics only carry a span, so this is +/// what puts `file:line:col` on an error inside an included or autoloaded file. +fn diagnostic_file(file: &Option, span: crate::span::Span) -> Option { + if file.is_some() { + return file.clone(); + } + if span.line == 0 { + return None; + } + span.source_path() +} + /// Builds the plain `kind[location]: message` diagnostic text shared by both /// `print_error` and `print_warning`, independent of decoration. fn format_diagnostic( @@ -98,6 +118,19 @@ mod tests { assert_eq!(msg, "warning[3:5]: bad"); } + /// A span lexed from a registered file names that file even when the error was not tagged. + #[test] + fn diagnostic_file_falls_back_to_the_span_source() { + let span = Span::new_in_source(4, 2, 9_001); + Span::register_source_path(9_001, "vendor/a/B.php"); + assert_eq!(diagnostic_file(&None, span).as_deref(), Some("vendor/a/B.php")); + assert_eq!( + diagnostic_file(&Some("x.php".to_string()), span).as_deref(), + Some("x.php") + ); + assert_eq!(diagnostic_file(&None, Span::new_in_source(4, 2, 9_002)), None); + } + /// Verifies diagnostics omit the location brackets when no location is available. #[test] fn format_diagnostic_no_location() { diff --git a/src/hash_prelude.rs b/src/hash_prelude.rs index 9d433749dc..75fba702e3 100644 --- a/src/hash_prelude.rs +++ b/src/hash_prelude.rs @@ -176,6 +176,14 @@ pub(crate) fn hash_declarations() -> Program { }) } +/// Whether `program` uses this prelude's surface: the same test `inject_if_used` applies. +/// +/// Also run by `crate::autoload::prelude_probe` over the autoload closure, whose files are only +/// loaded after this prelude has been decided. +pub(crate) fn program_uses_surface(program: &[crate::parser::ast::Stmt]) -> bool { + detect::program_uses_hash_context(program) +} + /// Injects the hash prelude when the program references the incremental-hashing /// surface, leaving every other program untouched. /// diff --git a/src/image_prelude.rs b/src/image_prelude.rs index 5695dc81ef..fa60cdd122 100644 --- a/src/image_prelude.rs +++ b/src/image_prelude.rs @@ -14377,6 +14377,14 @@ pub(crate) fn image_declarations() -> Program { }) } +/// Whether `program` uses this prelude's surface: the same test `inject_if_used` applies. +/// +/// Also run by `crate::autoload::prelude_probe` over the autoload closure, whose files are only +/// loaded after this prelude has been decided. +pub(crate) fn program_uses_surface(program: &[crate::parser::ast::Stmt]) -> bool { + detect::program_uses_image(program) +} + /// Prepends the image prelude to `program` when it references an image symbol, so /// the classes, constants, functions, and `elephc_image` externs compile through /// the normal pipeline only for image-using programs. The prelude carries only diff --git a/src/parser/mod.rs b/src/parser/mod.rs index ac99abfa49..6eb6389c98 100644 --- a/src/parser/mod.rs +++ b/src/parser/mod.rs @@ -65,6 +65,17 @@ pub(crate) fn next_anonymous_class_name() -> String { format!("class@anonymous#{}", id) } +/// Returns the next anonymous-class number, for [`restore_anonymous_class_counter`]. +pub(crate) fn anonymous_class_counter() -> usize { + ANONYMOUS_CLASS_COUNTER.with(|counter| counter.get()) +} + +/// Rewinds the anonymous-class numbering after a parse whose result is discarded (the autoload +/// prelude probe), so the real parse names its anonymous classes exactly as without it. +pub(crate) fn restore_anonymous_class_counter(next: usize) { + ANONYMOUS_CLASS_COUNTER.with(|counter| counter.set(next)); +} + /// Records a hoisted anonymous-class `ClassDecl` so the current parse appends it to the program. pub(crate) fn register_anonymous_class(decl: Stmt) { ANONYMOUS_CLASSES.with(|classes| classes.borrow_mut().push(decl)); diff --git a/src/pipeline.rs b/src/pipeline.rs index f676202a44..be8a8c84d4 100644 --- a/src/pipeline.rs +++ b/src/pipeline.rs @@ -237,6 +237,9 @@ fn compile_inner(config: CliConfig, entry: EntryKind<'_>) { process::exit(1); } }; + // The entry file lexes with the default source identity. Registered after the resolver, + // which rewinds the identity counter, so later diagnostics can name it (`Span::source_path`). + Span::register_source_path(0, filename.to_string()); let ast = match autoload::collect_aliases(ast) { Ok(ast) => ast, Err(error) => { @@ -265,6 +268,24 @@ fn compile_inner(config: CliConfig, entry: EntryKind<'_>) { process::exit(1); } + // The preludes below are decided on the entry program, but the autoload pass that loads the + // Composer class files runs after name resolution, much later. Probe that closure now, so a + // vendor file that uses PDO or curl gets its prelude as if the entry file used it. + crate::progress::phase("autoload-prelude-probe"); + let phase_started = Instant::now(); + let autoloaded_preludes = if entry.is_repl_host() { + HashSet::new() + } else { + autoload::prelude_probe::autoloaded_prelude_groups( + &ast, + parent, + &autoload_registry, + &defines, + ) + }; + timings.record_since("autoload-prelude-probe", phase_started); + let autoloaded = |group: &str| autoloaded_preludes.contains(group); + let mut prelude_inventory = optimize::reachability::PreludeInventory::new(); // `curl` belongs here for the same reason `pdo` does, and NOT listing it is a silent // no-op rather than a smaller binary: `--with-curl` force-injects the whole surface for @@ -308,7 +329,7 @@ fn compile_inner(config: CliConfig, entry: EntryKind<'_>) { // injects when `force || detect(...)`, so `force = pdo_used` reproduces the // exact decision without a second AST walk (the injected PDO prelude would // otherwise be re-scanned by the mysqli detection below too). - let pdo_used = pdo_force || pdo_prelude::program_uses_pdo(&ast); + let pdo_used = pdo_force || autoloaded("pdo") || pdo_prelude::program_uses_pdo(&ast); let ast = if php_version == crate::web_prelude::PhpVersion::default() { pdo_prelude::inject_if_used(ast, pdo_used, &mut prelude_inventory) } else { @@ -333,7 +354,8 @@ fn compile_inner(config: CliConfig, entry: EntryKind<'_>) { crate::progress::phase("mysqli-prelude"); let phase_started = Instant::now(); let mysqli_force = with_crates.contains("mysqli"); - let mysqli_used = mysqli_force || mysqli_prelude::program_uses_mysqli(&ast); + let mysqli_used = + mysqli_force || autoloaded("mysqli") || mysqli_prelude::program_uses_mysqli(&ast); let ast = mysqli_prelude::inject_if_used(ast, mysqli_used, php_version, &mut prelude_inventory); if mysqli_used { @@ -350,7 +372,7 @@ fn compile_inner(config: CliConfig, entry: EntryKind<'_>) { let phase_started = Instant::now(); let ast = tz_prelude::inject_if_used( ast, - with_crates.contains("tz"), + with_crates.contains("tz") || autoloaded("tz"), &mut prelude_inventory, ); timings.record_since("tz-prelude", phase_started); @@ -371,7 +393,11 @@ fn compile_inner(config: CliConfig, entry: EntryKind<'_>) { // before name resolution so the call resolves to the injected function. crate::progress::phase("var-export-prelude"); let phase_started = Instant::now(); - let ast = var_export_prelude::inject_if_used(ast, &mut prelude_inventory); + let ast = var_export_prelude::inject_if_used_or_referenced_elsewhere( + ast, + autoloaded("var_export"), + &mut prelude_inventory, + ); timings.record_since("var-export-prelude", phase_started); // Inject the OPcache preludes (pure elephc-PHP functions): `opcache_get_configuration()` @@ -463,7 +489,7 @@ fn compile_inner(config: CliConfig, entry: EntryKind<'_>) { let phase_started = Instant::now(); let ast = crate::image_prelude::inject_if_used( ast, - with_crates.contains("image"), + with_crates.contains("image") || autoloaded("image"), &mut prelude_inventory, ); timings.record_since("image-prelude", phase_started); @@ -476,7 +502,7 @@ fn compile_inner(config: CliConfig, entry: EntryKind<'_>) { // detected, and before name resolution so a namespaced caller resolves to it. crate::progress::phase("hash-prelude"); let phase_started = Instant::now(); - let ast = crate::hash_prelude::inject_if_used(ast, false, &mut prelude_inventory); + let ast = crate::hash_prelude::inject_if_used(ast, autoloaded("hash"), &mut prelude_inventory); let ast = crate::core_prelude::inject_if_used(ast, &mut prelude_inventory); let ast = crate::bcmath_prelude::inject_if_used(ast, with_crates.contains("bcmath"), &mut prelude_inventory); timings.record_since("hash-prelude", phase_started); @@ -502,13 +528,13 @@ fn compile_inner(config: CliConfig, entry: EntryKind<'_>) { let ast = if php_version == crate::php_version::PhpVersion::default() { crate::curl_prelude::inject_if_used( ast, - with_crates.contains("curl"), + with_crates.contains("curl") || autoloaded("curl"), &mut prelude_inventory, ) } else { crate::curl_prelude::inject_if_used_for_version( ast, - with_crates.contains("curl"), + with_crates.contains("curl") || autoloaded("curl"), php_version, &mut prelude_inventory, ) @@ -524,7 +550,7 @@ fn compile_inner(config: CliConfig, entry: EntryKind<'_>) { let phase_started = Instant::now(); let ast = crate::xml_prelude::inject_if_used( ast, - with_crates.contains("xml"), + with_crates.contains("xml") || autoloaded("xml"), &mut prelude_inventory, ); timings.record_since("xml-prelude", phase_started); diff --git a/src/resolver/engine_includes.rs b/src/resolver/engine_includes.rs index 33d014e345..a6e542f8cc 100644 --- a/src/resolver/engine_includes.rs +++ b/src/resolver/engine_includes.rs @@ -25,7 +25,7 @@ use super::include_path::fold_include_path; use super::state::ResolveState; /// Process-global counter producing unique hidden temporary names for value-position includes. -static VALUE_INCLUDE_COUNTER: AtomicUsize = AtomicUsize::new(0); +pub(super) static VALUE_INCLUDE_COUNTER: AtomicUsize = AtomicUsize::new(0); /// Where the value produced by an expression-position include must be delivered. pub(super) enum IncludeValueCapture { diff --git a/src/resolver/files.rs b/src/resolver/files.rs index 4675e5b7c9..673bb3181b 100644 --- a/src/resolver/files.rs +++ b/src/resolver/files.rs @@ -50,6 +50,7 @@ pub(super) fn parse_file( let mode = SourceMode::from_path(path); let source_id = Span::fresh_source_id(); + Span::register_source_path(source_id, file.clone()); let tokens = lexer::tokenize_with_mode_and_source_id(&source, mode, source_id) .map_err(|e| e.with_file(file.clone()))?; diff --git a/src/resolver/mod.rs b/src/resolver/mod.rs index e3074908bf..787c821b93 100644 --- a/src/resolver/mod.rs +++ b/src/resolver/mod.rs @@ -113,6 +113,42 @@ pub fn resolve_collecting_includes_with_defines( program: Program, base_dir: &Path, defines: &HashSet, +) -> Result<(Program, Vec), CompileError> { + crate::compiler_stack::with_compiler_stack(|| { + Span::reset_source_ids(); + resolve_collecting_includes_with_defines_inner(program, base_dir, defines) + }) +} + +/// The resolver's hidden-temporary counters at one moment; see [`counters_checkpoint`]. +#[derive(Debug, Clone, Copy)] +pub struct ResolverCounters([usize; 1]); + +/// Captures the counters that name the resolver's hidden temporaries, to hand back to +/// [`restore_counters`] after a resolution whose result is discarded (the autoload prelude +/// probe), so the real resolution names its temporaries exactly as without it. +pub fn counters_checkpoint() -> ResolverCounters { + use std::sync::atomic::Ordering::SeqCst; + ResolverCounters([engine_includes::VALUE_INCLUDE_COUNTER.load(SeqCst)]) +} + +/// Rewinds the resolver's hidden-temporary counters to a [`counters_checkpoint`]. +pub fn restore_counters(checkpoint: ResolverCounters) { + use std::sync::atomic::Ordering::SeqCst; + let [value_include] = checkpoint.0; + engine_includes::VALUE_INCLUDE_COUNTER.store(value_include, SeqCst); +} + +/// Resolves the includes of a file loaded INTO an ongoing compile (an autoloaded class file). +/// +/// Unlike [`resolve_collecting_includes_with_defines`] it keeps the source identities the +/// compile already handed out, so a file this pulls in never reuses the identity of another +/// file of the same program: spans of different files stay distinct, and each diagnostic can +/// name its own file. +pub fn resolve_collecting_includes_continuing( + program: Program, + base_dir: &Path, + defines: &HashSet, ) -> Result<(Program, Vec), CompileError> { crate::compiler_stack::with_compiler_stack(|| { resolve_collecting_includes_with_defines_inner(program, base_dir, defines) @@ -125,7 +161,6 @@ fn resolve_collecting_includes_with_defines_inner( base_dir: &Path, defines: &HashSet, ) -> Result<(Program, Vec), CompileError> { - Span::reset_source_ids(); if !has_includes(&program) { return Ok((program, Vec::new())); } diff --git a/src/span.rs b/src/span.rs index 1b21ba5200..835c24485a 100644 --- a/src/span.rs +++ b/src/span.rs @@ -98,6 +98,22 @@ std::thread_local! { /// one resolution unit, which is pathological). static NEXT_SYNTHETIC_SOURCE_ID: std::cell::Cell = const { std::cell::Cell::new(SOURCE_ID_MASK + 1) }; + /// The physical file each source identity was lexed from, for diagnostics. + /// + /// A checker error carries only a span; without this a diagnostic from an included or + /// autoloaded file printed `error[line:col]` with no way to tell which of thousands of files + /// it came from. Entries are overwritten when an identity is handed out again after + /// [`Span::reset_source_ids`], so a lookup always names the latest file lexed with that id. + static SOURCE_PATHS: std::cell::RefCell> = + std::cell::RefCell::new(std::collections::HashMap::new()); +} + +/// The span counters at one moment; see [`Span::counters_checkpoint`]. +#[derive(Debug, Clone, Copy)] +pub struct SpanCounters { + next_source_id: u32, + next_synthetic_source_id: u32, + next_synthetic_line: u32, } #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] @@ -168,11 +184,46 @@ impl Span { }) } + /// Records the physical file a source identity was lexed from. + pub fn register_source_path(source_id: u32, path: impl Into) { + let path = path.into(); + SOURCE_PATHS.with(|paths| { + paths.borrow_mut().insert(source_id, path); + }); + } + + /// Returns the physical file this span was lexed from, when one was registered. + pub fn source_path(self) -> Option { + let source_id = self.source_id(); + SOURCE_PATHS.with(|paths| paths.borrow().get(&source_id).cloned()) + } + /// Resets source identities at the beginning of a new include-resolution unit. pub fn reset_source_ids() { NEXT_SOURCE_ID.with(|next| next.set(1)); } + /// Captures every span counter (include and synthetic source identities, synthetic lines), + /// to hand back to [`Span::restore_counters`] after work whose spans are discarded. + pub fn counters_checkpoint() -> SpanCounters { + SpanCounters { + next_source_id: NEXT_SOURCE_ID.with(|next| next.get()), + next_synthetic_source_id: NEXT_SYNTHETIC_SOURCE_ID.with(|next| next.get()), + next_synthetic_line: NEXT_SYNTHETIC_LINE.load(std::sync::atomic::Ordering::Relaxed), + } + } + + /// Rewinds the span counters to a [`Span::counters_checkpoint`], so a discarded trial parse + /// (the autoload prelude probe) leaves the numbering the real pass will use. + pub fn restore_counters(checkpoint: SpanCounters) { + NEXT_SOURCE_ID.with(|next| next.set(checkpoint.next_source_id)); + NEXT_SYNTHETIC_SOURCE_ID.with(|next| next.set(checkpoint.next_synthetic_source_id)); + NEXT_SYNTHETIC_LINE.store( + checkpoint.next_synthetic_line, + std::sync::atomic::Ordering::Relaxed, + ); + } + /// Returns a source identity for a synthetic parse that [`Span::reset_source_ids`] must not /// rewind. /// diff --git a/src/types/checker/builtins/mod.rs b/src/types/checker/builtins/mod.rs index bb9ae0a298..84384d5a73 100644 --- a/src/types/checker/builtins/mod.rs +++ b/src/types/checker/builtins/mod.rs @@ -106,9 +106,11 @@ impl Checker { // eagerly inferred by argument normalization. Their handlers inspect the // raw operands directly. let builtin_key = crate::names::php_symbol_key(name.trim_start_matches('\\')); - if self.has_function_decl_folded(name) - && crate::builtins::registry::lookup(name).is_some() + // The declaration lookup scans every declared function, so it runs last: almost every + // builtin is available on the target, which settles the condition without it. + if crate::builtins::registry::lookup(name).is_some() && !catalog::builtin_is_available_for_target(name, self.target) + && self.has_function_decl_folded(name) { return Ok(None); } diff --git a/src/types/checker/driver/functions.rs b/src/types/checker/driver/functions.rs index f14f28e6fd..417df976f2 100644 --- a/src/types/checker/driver/functions.rs +++ b/src/types/checker/driver/functions.rs @@ -149,6 +149,12 @@ impl Checker { /// Returns true if `name` resolves to any declared function: user declaration, variant group, or /// extern. Resolution is case-insensitive via PHP symbol key matching. pub(crate) fn has_function_decl_folded(&self, name: &str) -> bool { + if self.fn_decls.contains_key(name) + || self.function_variant_groups.contains_key(name) + || self.extern_functions.contains_key(name) + { + return true; + } let key = php_symbol_key(name); self.fn_decls .keys() @@ -467,6 +473,11 @@ impl Checker { /// matching) key if one exists. Used to translate case-insensitive names to their actual declared /// spelling. fn folded_map_key(map: &HashMap, name: &str) -> Option { + // A spelling that is itself a key is the canonical one: a case-insensitive symbol table + // never holds two spellings of one name, so the scan below could only find this same key. + if map.contains_key(name) { + return Some(name.to_string()); + } let key = php_symbol_key(name); map.keys() .find(|existing| php_symbol_key(existing) == key) diff --git a/src/types/checker/driver/init.rs b/src/types/checker/driver/init.rs index dfcd529aef..eeec476d34 100644 --- a/src/types/checker/driver/init.rs +++ b/src/types/checker/driver/init.rs @@ -62,6 +62,9 @@ impl Checker { strict_types: false, internal_callback_binding: false, param_specialization_seen: HashSet::new(), + class_children_index: None, + method_impl_users_index: None, + property_sharers_index: None, interface_method_boxed_params: HashSet::new(), requested_instantiations: Vec::new(), generic_call_sites: HashMap::new(), @@ -132,6 +135,7 @@ impl Checker { boxed_reference_promotion_sites: HashMap::new(), first_class_builtin_call_types: HashMap::new(), loop_storage_types: HashMap::new(), + loop_storage_scope_spans: HashMap::new(), string_incdec_locals: HashSet::new(), strict_locals: false, local_conditional_depth: 0, diff --git a/src/types/checker/driver/mod.rs b/src/types/checker/driver/mod.rs index 080b5714d4..44fb8ba414 100644 --- a/src/types/checker/driver/mod.rs +++ b/src/types/checker/driver/mod.rs @@ -135,7 +135,7 @@ pub(super) fn check_types_impl( let key = php_symbol_key(&class.name); if !seen_classes.insert(key) { errors.push(CompileError::new( - crate::span::Span::dummy(), + class.span, &format!("Duplicate class declaration: {}", class.name), )); continue; @@ -574,6 +574,57 @@ mod tests { } } + /// The outer method/function/top-level fixpoint must converge when a method's untyped + /// variadic is both promoted to the descriptor container (a first-class callable inside a + /// method body) and called with a string from the top level. + /// + /// The method pass promoted `is`'s collector to `array`, the top-level re-check joined + /// the `'api/*'` call site back into `array` (`Str` absorbs `Mixed` in the coercion + /// join), and the two alternated until the round budget ran out; on Laravel that budget is + /// thousands of rounds. The collector must end as the wider `array`, the only type the + /// descriptor invoker and both call sites agree on. + #[test] + fn promoted_method_variadic_is_not_narrowed_back_by_a_top_level_call() { + let source = r#"is(...); + return $f("api/x"); + } +} +$g = fn (Req $r) => $r->is('api/*'); +var_dump($g(new Req())); +var_dump((new Mw())->check(new Req())); +"#; + let tokens = crate::lexer::tokenize(source).expect("tokenize"); + let program = crate::parser::parse(&tokens).expect("parse"); + let program = crate::name_resolver::resolve(program).expect("name resolve"); + let checked = crate::types::checker::check_types( + &program, + crate::codegen_support::platform::Target::new( + crate::codegen_support::platform::Platform::MacOS, + crate::codegen_support::platform::Arch::AArch64, + ), + ) + .expect("check"); + let is_sig = checked + .classes + .get("Req") + .and_then(|class| class.methods.get("is")) + .expect("Req::is signature"); + assert_eq!( + is_sig.params.last().map(|(_, ty)| ty.clone()), + Some(crate::types::PhpType::Array(Box::new(crate::types::PhpType::Mixed))), + "the promoted collector must stay array" + ); + } + /// Class ids must not depend on HashMap iteration order. /// /// THIS TEST CANNOT BE WRITTEN AS "check the same source twice and compare". Rust seeds its diff --git a/src/types/checker/functions/resolution/signature.rs b/src/types/checker/functions/resolution/signature.rs index b9ebd337d8..fa562f71ba 100644 --- a/src/types/checker/functions/resolution/signature.rs +++ b/src/types/checker/functions/resolution/signature.rs @@ -138,8 +138,7 @@ impl Checker { .collect(); let prev_by_ref_return = self.current_by_ref_return; self.current_by_ref_return = decl.by_ref_return; - self.loop_storage_types - .retain(|(scope, _), _| scope != &function_key); + self.clear_loop_storage_scope(&function_key); let previous_loop_storage_scope = std::mem::replace(&mut self.current_loop_storage_scope, function_key.clone()); self.resolving_functions.insert(function_key.clone()); diff --git a/src/types/checker/inference/expr/class_refs.rs b/src/types/checker/inference/expr/class_refs.rs index 2e131e9026..8076e55cf7 100644 --- a/src/types/checker/inference/expr/class_refs.rs +++ b/src/types/checker/inference/expr/class_refs.rs @@ -28,12 +28,9 @@ impl Checker { expr: &Expr, env: &TypeEnv, ) -> Result<(), CompileError> { - let mut class_names: Vec = self - .classes - .keys() - .filter(|name| self.class_is_same_or_descends_from(name, base_class)) - .cloned() - .collect(); + // Every registered class at or below `base_class`, from the children index rather than a + // parent-chain walk from every class in the table (`new static` sits in hot factories). + let mut class_names: Vec = self.class_and_descendants(base_class); class_names.sort(); for class_name in class_names { @@ -75,22 +72,6 @@ impl Checker { Ok(()) } - /// Checks whether `class_name` is either `base_class` itself or a descendant of it - /// by walking the parent chain. - fn class_is_same_or_descends_from(&self, class_name: &str, base_class: &str) -> bool { - let mut current = Some(class_name); - while let Some(name) = current { - if name == base_class { - return true; - } - current = self - .classes - .get(name) - .and_then(|info| info.parent.as_deref()); - } - false - } - /// Infers the type of a class constant or enum case accessed via scope resolution /// (e.g., `MyClass::CONSTANT` or `Color::Red`). /// diff --git a/src/types/checker/inference/objects/methods.rs b/src/types/checker/inference/objects/methods.rs index 2d0e9caf18..73cbc7beda 100644 --- a/src/types/checker/inference/objects/methods.rs +++ b/src/types/checker/inference/objects/methods.rs @@ -8,6 +8,8 @@ //! Key details: //! - Object inference depends on flattened class metadata, visibility, inheritance, and declared property types. +use std::collections::{HashMap, HashSet}; + use crate::errors::CompileError; use crate::names::php_symbol_key; use crate::parser::ast::{Expr, ExprKind, StaticReceiver, TypeExpr}; @@ -711,7 +713,7 @@ impl Checker { } if let Some((_, PhpType::Array(existing_elem_ty))) = sig.params.last_mut() { **existing_elem_ty = - wider_type_syntactic(existing_elem_ty.as_ref(), &elem_ty); + widen_method_variadic_element(existing_elem_ty.as_ref(), &elem_ty); } } return Ok(late_static_return_type @@ -722,6 +724,48 @@ impl Checker { Ok(PhpType::Int) } + /// Returns `ancestor` (when it is a registered class) and every registered class below it. + /// + /// The same set the parent-chain walk `class is ancestor or one of its subclasses` selects + /// over the whole table, found from the children index instead: a cycle in the parent + /// links is cut by the visited set, exactly where the walk's hop limit cut it. + pub(crate) fn class_and_descendants(&mut self, ancestor: &str) -> Vec { + let class_count = self.classes.len(); + if self + .class_children_index + .as_ref() + .is_none_or(|(built_for, _)| *built_for != class_count) + { + let mut children: HashMap> = HashMap::new(); + for (name, info) in &self.classes { + if let Some(parent) = &info.parent { + children.entry(parent.clone()).or_default().push(name.clone()); + } + } + self.class_children_index = Some((class_count, children)); + } + let children = &self + .class_children_index + .as_ref() + .expect("children index built above") + .1; + let mut found = Vec::new(); + let mut visited: HashSet<&str> = HashSet::new(); + let mut stack: Vec<&str> = vec![ancestor]; + while let Some(name) = stack.pop() { + if !visited.insert(name) { + continue; + } + if self.classes.contains_key(name) { + found.push(name.to_string()); + } + if let Some(direct) = children.get(name) { + stack.extend(direct.iter().map(String::as_str)); + } + } + found + } + /// Returns preserved late-static return syntax for an instance method. fn instance_method_late_static_return( &self, @@ -1387,7 +1431,7 @@ impl Checker { } if let Some((_, PhpType::Array(existing_elem_ty))) = sig.params.last_mut() { **existing_elem_ty = - wider_type_syntactic(existing_elem_ty.as_ref(), &elem_ty); + widen_method_variadic_element(existing_elem_ty.as_ref(), &elem_ty); } } return Ok(late_static_static_return_type @@ -1452,7 +1496,7 @@ impl Checker { } if let Some((_, PhpType::Array(existing_elem_ty))) = sig.params.last_mut() { **existing_elem_ty = - wider_type_syntactic(existing_elem_ty.as_ref(), &elem_ty); + widen_method_variadic_element(existing_elem_ty.as_ref(), &elem_ty); } } return Ok(late_static_instance_return_type @@ -1577,6 +1621,24 @@ fn method_variadic_tail_needs_iterable( }) } +/// Joins a call site's argument type into a method's untyped variadic element type. +/// +/// MONOTONE: the stored element only ever widens. `wider_type_syntactic` follows PHP's coercion +/// order, in which `Str` absorbs every other arm, so on its own it turned a `mixed` element back +/// into `string` whenever a string argument arrived. `mixed` is exactly what a first-class +/// callable or callable array promotes the collector to (the descriptor container, +/// `crate::types::signatures::descriptor_variadic_container`), so the method pass promoted it, +/// the top-level re-check of a string call site narrowed it again, and the outer fixpoint +/// alternated between the two until its pass budget ran out (Laravel's +/// `Illuminate\Http\Request::is(...$patterns)`: thousands of rounds). A `mixed` element holds any +/// argument already, so it stays. +fn widen_method_variadic_element(existing: &PhpType, incoming: &PhpType) -> PhpType { + if *existing == PhpType::Mixed { + return PhpType::Mixed; + } + wider_type_syntactic(existing, incoming) +} + /// Returns whether a method signature stores its variadic slot by reference. fn method_variadic_param_is_by_ref(sig: &FunctionSig) -> bool { let Some(variadic_name) = sig.variadic.as_ref() else { diff --git a/src/types/checker/inference/ops.rs b/src/types/checker/inference/ops.rs index 657273c9e7..d2603b2409 100644 --- a/src/types/checker/inference/ops.rs +++ b/src/types/checker/inference/ops.rs @@ -522,8 +522,7 @@ impl Checker { &previous_loop_storage_scope, expr.span, ); - self.loop_storage_types - .retain(|(scope, _), _| scope != &loop_storage_scope); + self.clear_loop_storage_scope(&loop_storage_scope); self.current_loop_storage_scope = loop_storage_scope; // The storage this closure's frame ALREADY holds on entry: its by-VALUE captures, each // with the type it arrives with, read from the ENCLOSING environment. Deliberately not diff --git a/src/types/checker/method_pass.rs b/src/types/checker/method_pass.rs index b88a9f966a..047b733bde 100644 --- a/src/types/checker/method_pass.rs +++ b/src/types/checker/method_pass.rs @@ -218,8 +218,7 @@ impl Checker { self.current_method_is_static = method.is_static; self.current_by_ref_return = method.by_ref_return; let loop_storage_scope = format!("{}::{}", class.name, method.name); - self.loop_storage_types - .retain(|(scope, _), _| scope != &loop_storage_scope); + self.clear_loop_storage_scope(&loop_storage_scope); let previous_loop_storage_scope = std::mem::replace( &mut self.current_loop_storage_scope, loop_storage_scope, @@ -496,15 +495,14 @@ impl Checker { // A subclass copies the method signature before body inference runs. Calls through // that subclass must use the implementing method's return ABI, or a boxed result can // be read as a scalar even though both names dispatch to the same body. - for (name, ci) in &mut self.classes { - let implementation = if method.is_static { - ci.static_method_impl_classes.get(&method_key) - } else { - ci.method_impl_classes.get(&method_key) - }; - if name != &class.name && implementation != Some(&class.name) { + let mut receivers = self.method_impl_users(&class.name, &method_key, method.is_static); + if !receivers.iter().any(|name| name == &class.name) { + receivers.push(class.name.clone()); + } + for name in receivers { + let Some(ci) = self.classes.get_mut(&name) else { continue; - } + }; let sig = if method.is_static { ci.static_methods.get_mut(&method_key) } else { @@ -524,6 +522,47 @@ impl Checker { ); } + /// The classes whose `method_key` dispatches to `impl_class`'s body (see + /// `Checker::method_impl_users_index`). + fn method_impl_users( + &mut self, + impl_class: &str, + method_key: &str, + is_static: bool, + ) -> Vec { + let class_count = self.classes.len(); + if self + .method_impl_users_index + .as_ref() + .is_none_or(|(built_for, _)| *built_for != class_count) + { + let mut index: std::collections::HashMap<(String, String, bool), Vec> = + std::collections::HashMap::new(); + for (name, info) in &self.classes { + for (key, implementation) in &info.method_impl_classes { + index + .entry((implementation.clone(), key.clone(), false)) + .or_default() + .push(name.clone()); + } + for (key, implementation) in &info.static_method_impl_classes { + index + .entry((implementation.clone(), key.clone(), true)) + .or_default() + .push(name.clone()); + } + } + self.method_impl_users_index = Some((class_count, index)); + } + self.method_impl_users_index + .as_ref() + .and_then(|(_, index)| { + index.get(&(impl_class.to_string(), method_key.to_string(), is_static)) + }) + .cloned() + .unwrap_or_default() + } + /// Resolves the return type of a method whose body contains `yield`. /// /// The result is always `Generator`, because that is the object PHP hands back when @@ -573,15 +612,14 @@ impl Checker { let callable_array_sig = is_callable_array_return_type(return_type) .then(|| matching_callable_sig(callable_array_return_sigs)) .flatten(); - for (name, class_info) in &mut self.classes { - let implementation = if is_static { - class_info.static_method_impl_classes.get(method_key) - } else { - class_info.method_impl_classes.get(method_key) - }; - if name != class_name && implementation.map(String::as_str) != Some(class_name) { + let mut receivers = self.method_impl_users(class_name, method_key, is_static); + if !receivers.iter().any(|name| name == class_name) { + receivers.push(class_name.to_string()); + } + for name in receivers { + let Some(class_info) = self.classes.get_mut(&name) else { continue; - } + }; if let Some(sig) = &callable_sig { class_info .callable_method_return_sigs diff --git a/src/types/checker/mod.rs b/src/types/checker/mod.rs index 2ef37978be..39e31759d1 100644 --- a/src/types/checker/mod.rs +++ b/src/types/checker/mod.rs @@ -133,6 +133,25 @@ pub(crate) struct Checker { /// widen the parameter to `Mixed` (so e.g. a parameter called with both an int /// and a string is `Mixed`, not collapsed to one type). pub param_specialization_seen: HashSet<(String, usize)>, + /// Direct subclasses of every class, keyed by parent name, plus the `classes.len()` it was + /// built for. Rebuilt whenever a class is registered, so it never answers for a stale table. + /// + /// `class_and_descendants` answers subclass queries from it (the late-bound `new static` + /// target validation), which used to walk the parent chain from every registered class. + pub class_children_index: Option<(usize, HashMap>)>, + /// The classes whose method table dispatches each `(implementing class, method key, + /// is_static)` to that implementation, plus the `classes.len()` it was built for. + /// + /// Every method body's inferred return is copied to those classes after the body is + /// checked. That used to scan the whole class table once per method body (methods × + /// classes per pass). Implementation maps are only written while classes are registered, + /// which also changes the table's size, so the length is a sound staleness check. + pub method_impl_users_index: Option<(usize, HashMap<(String, String, bool), Vec>)>, + /// The classes sharing each `(property, declaring class)` slot, plus the `classes.len()` it + /// was built for; the same staleness rule as `method_impl_users_index`. A constructor + /// argument retypes that slot in every class sharing it, which used to scan the whole class + /// table per argument of every `new`. + pub property_sharers_index: Option<(usize, HashMap<(String, String), Vec>)>, /// Untyped interface positions whose implementation uses a stable boxed ABI. pub interface_method_boxed_params: HashSet<(String, usize)>, /// Every generic instantiation a call site asked for, as (template name, type arguments). @@ -437,6 +456,10 @@ pub(crate) struct Checker { pub first_class_builtin_call_types: HashMap, /// Fixed-point storage contracts keyed by function-like scope and loop span. pub loop_storage_types: crate::types::LoopStorageTypes, + /// The loop spans `loop_storage_types` holds for each scope, so a re-walk clears its scope + /// in time proportional to that scope's loops. Clearing used to `retain` over the whole map + /// once per method, function and closure body, which on Laravel was quadratic. + pub loop_storage_scope_spans: HashMap>, /// `(scope, local)` pairs for `string` locals used as a `++`/`--` target. /// /// PHP's string increment can change the value's type (`"9"++` is `int(10)`), so EIR diff --git a/src/types/checker/schema/classes/mod.rs b/src/types/checker/schema/classes/mod.rs index 9053f518c7..a49c8d7bc1 100644 --- a/src/types/checker/schema/classes/mod.rs +++ b/src/types/checker/schema/classes/mod.rs @@ -57,7 +57,10 @@ pub(crate) fn build_class_info_recursive( if !building.insert(class_name.to_string()) { return Err(CompileError::new( - crate::span::Span::dummy(), + class_map + .get(class_name) + .map(|class| class.span) + .unwrap_or_else(crate::span::Span::dummy), &format!( "Circular inheritance detected involving class {}", class_name @@ -66,6 +69,51 @@ pub(crate) fn build_class_info_recursive( } let class = load_class(class_name, class_map)?; + build_loaded_class_info(class_name, class, class_map, checker, next_class_id, building) + .map_err(|error| locate_at_declaration(error, class_span_of(class_name, class_map))) +} + +/// The declaration span of `class_name`, when the program declares it. +fn class_span_of( + class_name: &str, + class_map: &HashMap, +) -> crate::span::Span { + class_map + .get(class_name) + .map(|class| class.span) + .unwrap_or_else(crate::span::Span::dummy) +} + +/// Points every location-less error of one class's schema build at the class declaration. +/// +/// The schema checks report against `Span::dummy()`, so on a program of thousands of classes +/// `error: Class A cannot extend final class B` named neither a file nor a line. The class +/// being built is the declaration the error is about; an error a parent's build already +/// located keeps its own position. +fn locate_at_declaration(mut error: CompileError, span: crate::span::Span) -> CompileError { + if span.line == 0 { + return error; + } + if error.span.line == 0 && error.file.is_none() { + error.span = span; + } + error.related = error + .related + .into_iter() + .map(|related| locate_at_declaration(related, span)) + .collect(); + error +} + +/// [`build_class_info_recursive`] for a class whose declaration is loaded and not yet building. +fn build_loaded_class_info( + class_name: &str, + class: FlattenedClass, + class_map: &HashMap, + checker: &mut Checker, + next_class_id: &mut u64, + building: &mut HashSet, +) -> Result<(), CompileError> { validate_class_modifiers(&class)?; let parent_info = resolve_parent_info( class_name, diff --git a/src/types/checker/stmt_check/control_flow.rs b/src/types/checker/stmt_check/control_flow.rs index 21778c0d4a..9cc7cf548e 100644 --- a/src/types/checker/stmt_check/control_flow.rs +++ b/src/types/checker/stmt_check/control_flow.rs @@ -31,6 +31,17 @@ const FS_CURRENT_AS_PATHNAME: i64 = 32; const FS_CURRENT_MODE_MASK: i64 = 240; const FS_SKIP_DOTS: i64 = 4096; +impl Checker { + /// Forgets every loop-storage contract recorded for `scope`, before its body is walked again. + pub(crate) fn clear_loop_storage_scope(&mut self, scope: &str) { + if let Some(spans) = self.loop_storage_scope_spans.remove(scope) { + for span in spans { + self.loop_storage_types.remove(&(scope.to_string(), span)); + } + } + } +} + /// Computes and records fixed-point array storage contracts before checking a loop body. /// /// The shared analysis iterates over rebinds and growth sites with an evolving environment, so @@ -86,7 +97,15 @@ fn stabilize_loop_storage( Some(inferred) }, ); - let recorded = checker.loop_storage_types.entry(key).or_default(); + let (scope, span) = key; + if !checker.loop_storage_types.contains_key(&(scope.clone(), span)) { + checker + .loop_storage_scope_spans + .entry(scope.clone()) + .or_default() + .push(span); + } + let recorded = checker.loop_storage_types.entry((scope, span)).or_default(); for (name, storage_type) in contracts { recorded.insert(name.clone(), storage_type.clone()); env.insert(name, storage_type); diff --git a/src/types/checker/type_compat/object_types.rs b/src/types/checker/type_compat/object_types.rs index d75457bad8..518ab9a653 100644 --- a/src/types/checker/type_compat/object_types.rs +++ b/src/types/checker/type_compat/object_types.rs @@ -359,6 +359,34 @@ impl Checker { None } + /// The classes whose `prop_name` slot is the one `declaring_class` declares (see + /// `Checker::property_sharers_index`). + fn property_sharers(&mut self, prop_name: &str, declaring_class: &str) -> Vec { + let class_count = self.classes.len(); + if self + .property_sharers_index + .as_ref() + .is_none_or(|(built_for, _)| *built_for != class_count) + { + let mut index: std::collections::HashMap<(String, String), Vec> = + std::collections::HashMap::new(); + for (name, info) in &self.classes { + for (property, owner) in &info.property_declaring_classes { + index + .entry((property.clone(), owner.clone())) + .or_default() + .push(name.clone()); + } + } + self.property_sharers_index = Some((class_count, index)); + } + self.property_sharers_index + .as_ref() + .and_then(|(_, index)| index.get(&(prop_name.to_string(), declaring_class.to_string()))) + .cloned() + .unwrap_or_default() + } + /// Propagates a resolved constructor argument type into the corresponding constructor /// parameter and property slot for all classes that share an inherited property from /// `declaring_class`. Used to sharpen types across an inheritance hierarchy after @@ -396,15 +424,10 @@ impl Checker { return; }; - for class_info in self.classes.values_mut() { - let shares_inherited_property = class_info - .property_declaring_classes - .get(&prop_name) - .is_some_and(|owner| owner == &declaring_class); - - if !shares_inherited_property { + for sharer in self.property_sharers(&prop_name, &declaring_class) { + let Some(class_info) = self.classes.get_mut(&sharer) else { continue; - } + }; let property_has_declared_type = class_info.visible_property_is_declared(&prop_name); if !property_has_declared_type { diff --git a/src/tz_prelude.rs b/src/tz_prelude.rs index 5f4ca07a87..2f515b4a50 100644 --- a/src/tz_prelude.rs +++ b/src/tz_prelude.rs @@ -82,6 +82,14 @@ fn t_datetimezone() -> TypeExpr { crate::synthetic_class::t_class("DateTimeZone") } +/// Whether `program` uses this prelude's surface: the same test `inject_if_used` applies. +/// +/// Also run by `crate::autoload::prelude_probe` over the autoload closure, whose files are only +/// loaded after this prelude has been decided. +pub(crate) fn program_uses_surface(program: &[crate::parser::ast::Stmt]) -> bool { + detect::program_uses_tz_introspection(program) +} + /// Prepends the timezone-introspection prelude to `program` when it references the /// introspection surface, so the `elephc_tz` externs and helper functions compile /// through the normal pipeline only for programs that use them. The prelude is diff --git a/src/var_export_prelude.rs b/src/var_export_prelude.rs index f9b838b8a8..8f7b7912e2 100644 --- a/src/var_export_prelude.rs +++ b/src/var_export_prelude.rs @@ -424,21 +424,39 @@ pub const ECHO_HELPER: &str = "__elephc_var_export_echo"; /// Runs under `crate::compiler_stack::with_compiler_stack` (issue #1149): its usage scan walks the /// whole program, so an embedder calling it without the CLI driver's wrapper gets the same stack /// budget. +#[allow(dead_code)] // The CLI calls the `_or_referenced_elsewhere` form; embedders and tests call this. pub fn inject_if_used( program: Program, inventory: &mut crate::optimize::reachability::PreludeInventory, +) -> Program { + inject_if_used_or_referenced_elsewhere(program, false, inventory) +} + +/// [`inject_if_used`], also injecting when `referenced_elsewhere` says a file the autoload pass +/// loads later calls `var_export` (`crate::autoload::prelude_probe`). A program that declares +/// its own `var_export` still gets nothing. +pub fn inject_if_used_or_referenced_elsewhere( + program: Program, + referenced_elsewhere: bool, + inventory: &mut crate::optimize::reachability::PreludeInventory, ) -> Program { crate::compiler_stack::with_compiler_stack(|| { - inject_if_used_on_compiler_stack(program, inventory) + inject_if_used_on_compiler_stack(program, referenced_elsewhere, inventory) }) } +/// Whether `program` calls `var_export` without declaring its own. +pub(crate) fn program_uses_surface(program: &[crate::parser::ast::Stmt]) -> bool { + detect::program_references_var_export(program) && !detect::program_declares_var_export(program) +} + /// The body of [`inject_if_used`], run on the stack `with_compiler_stack` sized. fn inject_if_used_on_compiler_stack( program: Program, + referenced_elsewhere: bool, inventory: &mut crate::optimize::reachability::PreludeInventory, ) -> Program { - if !detect::program_references_var_export(&program) + if !(referenced_elsewhere || detect::program_references_var_export(&program)) || detect::program_declares_var_export(&program) { return program; diff --git a/src/xml_prelude.rs b/src/xml_prelude.rs index 916e865195..9902cf07f8 100644 --- a/src/xml_prelude.rs +++ b/src/xml_prelude.rs @@ -44,6 +44,14 @@ pub(crate) fn xml_declarations() -> Program { build::xml_declarations() } +/// Whether `program` uses this prelude's surface: the same test `inject_if_used` applies. +/// +/// Also run by `crate::autoload::prelude_probe` over the autoload closure, whose files are only +/// loaded after this prelude has been decided. +pub(crate) fn program_uses_surface(program: &[crate::parser::ast::Stmt]) -> bool { + detect::program_uses_xml(program) +} + /// Injects the xml prelude when the program references the `ext/xml` / `ext/xmlwriter` /// surface, leaving every other program untouched. /// diff --git a/tests/autoload_diagnostics_tests.rs b/tests/autoload_diagnostics_tests.rs new file mode 100644 index 0000000000..d708cbe9dd --- /dev/null +++ b/tests/autoload_diagnostics_tests.rs @@ -0,0 +1,170 @@ +//! Purpose: +//! End-to-end tests for what the CLI does with the Composer AUTOLOAD CLOSURE of a program: the +//! file each diagnostic names, and the standard-library preludes a class file reached only +//! through autoload needs. +//! +//! Called from: +//! - `cargo test --test autoload_diagnostics_tests` through Rust's test harness. +//! +//! Key details: +//! - Autoloaded class files are loaded after name resolution, long after the entry file was +//! lexed and scanned. Two things used to miss them: +//! - every checker diagnostic in such a file printed `error[line:col]`, naming no file, so on a +//! large vendor tree the error could not be located; +//! - the PDO/curl/... preludes were decided on the entry file alone, so a class file typed +//! against `PDO` failed with `Unknown type: PDO` unless `--with-pdo` forced it. +//! - Tests invoke the elephc CLI (CARGO_BIN_EXE_elephc) with `--check` as a subprocess in an +//! isolated temp dir, so nothing is assembled or linked. + +use std::fs; +use std::path::{Path, PathBuf}; +use std::process::{Command, Output}; +use std::sync::atomic::{AtomicUsize, Ordering}; + +static TEST_ID: AtomicUsize = AtomicUsize::new(0); + +/// Writes `files` into a fresh canonicalized temp dir and returns it. +fn project(prefix: &str, files: &[(&str, &str)]) -> PathBuf { + let id = TEST_ID.fetch_add(1, Ordering::SeqCst); + let dir = std::env::temp_dir().join(format!( + "{}_{}_{:?}_{}", + prefix, + std::process::id(), + std::thread::current().id(), + id + )); + fs::create_dir_all(&dir).unwrap(); + let dir = dir.canonicalize().unwrap(); + for (path, content) in files { + let full = dir.join(path); + fs::create_dir_all(full.parent().unwrap()).unwrap(); + fs::write(full, content).unwrap(); + } + dir +} + +/// Resolves the elephc CLI binary path (cargo env var, fallback next to the test binary). +fn elephc_bin() -> String { + std::env::var("CARGO_BIN_EXE_elephc").unwrap_or_else(|_| { + let mut path = std::env::current_exe().expect("failed to resolve current test binary"); + path.pop(); + if path.ends_with("deps") { + path.pop(); + } + path.join("elephc").to_string_lossy().into_owned() + }) +} + +/// Runs `elephc --check` on `/main.php`. +fn check(dir: &Path) -> Output { + Command::new(elephc_bin()) + .env("XDG_CACHE_HOME", dir.join("cache-root")) + .current_dir(dir) + .arg("--check") + .arg(dir.join("main.php")) + .output() + .expect("failed to spawn elephc") +} + +const COMPOSER: &str = r#"{"autoload":{"psr-4":{"App\\":"src/"}}}"#; + +/// A checker error inside an autoloaded class file names that file, not just a line. +#[test] +fn checker_error_in_an_autoloaded_file_names_the_file() { + let dir = project( + "elephc_autoload_diag_path", + &[ + ("composer.json", COMPOSER), + ( + "src/Broken.php", + "run();\n"), + ], + ); + let output = check(&dir); + let stderr = String::from_utf8_lossy(&output.stderr); + assert!(!output.status.success(), "expected a checker error, got success"); + let expected = format!("error[{}:4:", dir.join("src/Broken.php").display()); + assert!( + stderr.contains(&expected), + "diagnostic should name the autoloaded file ({expected}), got:\n{stderr}" + ); + let _ = fs::remove_dir_all(&dir); +} + +/// A checker error in the entry file names the entry file too. +#[test] +fn checker_error_in_the_entry_file_names_the_file() { + let dir = project( + "elephc_entry_diag_path", + &[("main.php", "pdo !== null || $this->curl !== null; }\n}\n", + ), + ( + "main.php", + "connected() ? \"yes\" : \"no\";\n", + ), + ], + ); + let output = check(&dir); + let stderr = String::from_utf8_lossy(&output.stderr); + assert!( + output.status.success(), + "an autoloaded PDO/curl user should check without --with-*, got:\n{stderr}" + ); + let _ = fs::remove_dir_all(&dir); +} + +/// Two autoloaded class files whose retyping assignments sit at the same line and column are +/// told apart: each file has its own source identity, so the per-position binding decisions no +/// longer collide ("Cannot re-bind $x here", issue #778) and both methods run. +#[test] +fn autoloaded_files_with_retypes_at_the_same_position_compile() { + let class = |name: &str, first: &str, second: &str| { + format!( + "f();\n(new App\\B())->f();\n"), + ], + ); + let output = check(&dir); + let stderr = String::from_utf8_lossy(&output.stderr); + assert!( + output.status.success(), + "same-position retypes in two autoloaded files should check, got:\n{stderr}" + ); + assert!(!stderr.contains("Cannot re-bind"), "unexpected re-bind error:\n{stderr}"); + let _ = fs::remove_dir_all(&dir); +} diff --git a/tests/codegen/callables/state_and_variadics.rs b/tests/codegen/callables/state_and_variadics.rs index ba208d8f23..0c0b6c014e 100644 --- a/tests/codegen/callables/state_and_variadics.rs +++ b/tests/codegen/callables/state_and_variadics.rs @@ -1035,3 +1035,30 @@ echo $ok ? "yes" : "no"; ); assert_eq!(out, "42|yes"); } + +/// An untyped method variadic that a first-class callable promotes to the mixed collector stays +/// mixed when a direct call later passes only strings. The direct string call used to narrow the +/// collector back to `array` while the callable path still stored mixed values into it, +/// and the program aborted with "heap memory exhausted". +#[test] +fn test_promoted_method_variadic_is_not_narrowed_back_by_a_string_call() { + let out = compile_and_run( + r#"is(...); + return $f("api/x", [1, 2], 3); + } +} +echo (new Req())->is('api/*'), "\n"; +echo (new Mw())->check(new Req()), "\n"; +"#, + ); + assert_eq!(out, "string:api/* 1\nstring:api/x array:[1,2] integer:3 3\n"); +} From 36f728cf6c85419e98a135f2b154dcc851889a57 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Joaqu=C3=ADn=20Gamboa?= Date: Tue, 20 Oct 2026 02:09:46 -0300 Subject: [PATCH 2/6] fix(diagnostics): name the entry file in include-resolution errors The entry file's source path was registered only after include resolution succeeded, so a missing require in the entry file still printed the file-less error[line:col]. Source id 0 is never handed out again by the resolver, so the path is now registered before it runs. --- src/pipeline.rs | 10 ++++++---- tests/autoload_diagnostics_tests.rs | 20 ++++++++++++++++++++ 2 files changed, 26 insertions(+), 4 deletions(-) diff --git a/src/pipeline.rs b/src/pipeline.rs index be8a8c84d4..0581f3e8e6 100644 --- a/src/pipeline.rs +++ b/src/pipeline.rs @@ -226,6 +226,11 @@ fn compile_inner(config: CliConfig, entry: EntryKind<'_>) { crate::progress::phase("resolve"); let phase_started = Instant::now(); + // The entry file lexes with the default source identity 0, which the resolver never hands + // out again (it rewinds the include counter to 1, and that does not touch the path registry). + // Registered before the resolver so an include error in the entry file names it too + // (`error[main.php:2:1]: Required file not found`). + Span::register_source_path(0, filename.to_string()); // `resolve_collecting_includes` also hands back the canonical path of every file the // resolver statically inlined — group 2 of the OPcache script manifest. let (ast, opcache_included_files) = @@ -237,10 +242,7 @@ fn compile_inner(config: CliConfig, entry: EntryKind<'_>) { process::exit(1); } }; - // The entry file lexes with the default source identity. Registered after the resolver, - // which rewinds the identity counter, so later diagnostics can name it (`Span::source_path`). - Span::register_source_path(0, filename.to_string()); - let ast = match autoload::collect_aliases(ast) { + let ast =match autoload::collect_aliases(ast) { Ok(ast) => ast, Err(error) => { crate::progress::clear(); diff --git a/tests/autoload_diagnostics_tests.rs b/tests/autoload_diagnostics_tests.rs index d708cbe9dd..bf7fb2f899 100644 --- a/tests/autoload_diagnostics_tests.rs +++ b/tests/autoload_diagnostics_tests.rs @@ -111,6 +111,26 @@ fn checker_error_in_the_entry_file_names_the_file() { let _ = fs::remove_dir_all(&dir); } +/// An include-resolution error in the entry file names the entry file as well: the path is +/// registered before the resolver runs, so `require 'missing.php'` reports +/// `error[/main.php:2:1]` instead of the file-less `error[2:1]`. +#[test] +fn missing_require_in_the_entry_file_names_the_file() { + let dir = project( + "elephc_entry_missing_require", + &[("main.php", " Date: Wed, 21 Oct 2026 18:39:27 -0300 Subject: [PATCH 3/6] docs: describe file-qualified diagnostics, the autoload prelude probe and checker indexes The compiler guides now cover what the README already showed: the kind[file:line:col] diagnostic format in docs/compiling, the autoload prelude probe and per-file source identities in the pipeline guide, and the span-to-file registry and class-table lookup indexes in the type checker internals. Stale error[line:col] examples are updated. --- docs/compiling/cli-reference.md | 2 +- docs/compiling/compilation-pipeline.md | 10 +++++++++- docs/compiling/output-and-diagnostics.md | 17 +++++++++++++++++ docs/internals/the-type-checker.md | 13 +++++++++++++ docs/php/types.md | 6 +++--- 5 files changed, 43 insertions(+), 5 deletions(-) diff --git a/docs/compiling/cli-reference.md b/docs/compiling/cli-reference.md index fe54a0e6be..3105839005 100644 --- a/docs/compiling/cli-reference.md +++ b/docs/compiling/cli-reference.md @@ -415,7 +415,7 @@ have run under: ``` $ elephc --php-version 8.2 app.php -error[3:5]: this program needs PHP 8.4 (property hooks), but --php-version selected 8.2; a binary built for 8.2 could not have run this source +error[app.php:3:5]: this program needs PHP 8.4 (property hooks), but --php-version selected 8.2; a binary built for 8.2 could not have run this source ``` Detected today: the pipe operator `|>` (8.5), property hooks and asymmetric diff --git a/docs/compiling/compilation-pipeline.md b/docs/compiling/compilation-pipeline.md index 2fe287b972..406d5a02f8 100644 --- a/docs/compiling/compilation-pipeline.md +++ b/docs/compiling/compilation-pipeline.md @@ -86,7 +86,15 @@ Physical source (.php or .lfc) when referenced. The web runtime prelude is injected with `--web`, and namespace/`use` rules rewrite references to fully-qualified - names. Autoloading is wired in around these steps. + names. Autoloading is wired in around these steps. Because the autoload pass + runs after name resolution, a prelude needed only by an autoloaded file would + be missed; when autoload rules exist, a probe (`src/autoload/prelude_probe.rs`) + first runs name resolution and the autoload pass on a copy of the program, + asks each prelude's detector about the result, and rewinds every counter it + advanced, so the real pass injects those preludes and numbers its files, + classes and temporaries exactly as it would without the probe. Every included + and autoloaded file gets its own source identity, which is how diagnostics + name the file they come from. - **func-args** — rewrites `func_num_args()`, `func_get_args()`, and `func_get_arg()` into a hidden variadic parameter plus ordinary PHP operations. This happens after autoloading but before manifest baking, optimization, and diff --git a/docs/compiling/output-and-diagnostics.md b/docs/compiling/output-and-diagnostics.md index f3c1744ef4..fa675d6e5d 100644 --- a/docs/compiling/output-and-diagnostics.md +++ b/docs/compiling/output-and-diagnostics.md @@ -112,6 +112,23 @@ the phase finishes, elephc keeps its action-oriented label with a checkmark and elapsed time, then starts the next phase on a new line. Non-interactive output and `--quiet` keep the compact plain output without progress lines. +### Diagnostic format + +Errors and warnings name the physical file, line and column they come from: + +```text +error[main.php:3:1]: Undefined variable: $x +error[main.php:2:1]: Required file not found: 'missing.php' +warning[src/Helpers.php:14:9]: Unreachable code +``` + +The file is the entry file as given on the command line, or the included or +autoloaded file that holds the statement, so a diagnostic in a large autoloaded +tree points at the right file. A position whose source file is not known (a +compiler-synthesized node) prints `kind[line:col]`, and a diagnostic with no +source position at all (some whole-program checks) prints `kind: message`. +Profile `note[…]` lines keep their `note[line:col]` form. + ### `--quiet` / `-q` Disables live and completed progress lines and forces unstyled plain output. It diff --git a/docs/internals/the-type-checker.md b/docs/internals/the-type-checker.md index d1a3ccb7a0..ebb461cc7d 100644 --- a/docs/internals/the-type-checker.md +++ b/docs/internals/the-type-checker.md @@ -728,6 +728,19 @@ After successful checking, elephc also runs a warning pass over the AST (`src/ty Warnings are returned through `CheckResult` and printed by the CLI without failing the compilation. +Checker errors and warnings carry only a `Span`. Each file the resolver or the autoload pass lexes gets its own source identity, and `Span::register_source_path` records which physical file that identity came from; `print_error` / `print_warning` fall back to `Span::source_path` when a diagnostic was not tagged with a file, so every positioned diagnostic prints as `kind[file:line:col]`. Class schema errors (circular inheritance, extending a `final` class, duplicate or unknown parent classes) point at the class declaration. + +### Lookup indexes + +Large programs (thousands of classes) made several per-member scans of the whole class table dominate checking time. The checker keeps lazily built indexes for them, each stored with the `classes.len()` it was built for, so registering a class invalidates it: + +- `class_children_index`: direct subclasses per parent, used by `class_and_descendants` for late-bound `new static` targets. +- `method_impl_users_index`: the classes whose method table dispatches a given `(implementing class, method, is_static)` to that implementation, to which each checked body's inferred return is copied. +- `property_sharers_index`: the classes sharing a `(property, declaring class)` slot, retyped together by a constructor argument. +- `loop_storage_scope_spans`: the loop spans `loop_storage_types` holds for each scope, so re-walking a scope clears only its own loop contracts. + +Builtin, function and folded-map lookups try the exact key before the case-insensitive scan. + ## Where the checker sits in the optimizer pipeline The type checker sits between an early folding pass and four post-check cleanup passes in `src/optimize/`: diff --git a/docs/php/types.md b/docs/php/types.md index c3a0bbafa2..63bb12d134 100644 --- a/docs/php/types.md +++ b/docs/php/types.md @@ -527,11 +527,11 @@ These standard PHP filesystem functions are intentionally absent from elephc bec ### Compiler diagnostics -elephc reports errors with source spans. Example: +elephc reports errors with the file, line and column they come from. Example: ```text -error[3:5]: Undefined variable: $name -error[8:1]: Function 'foo' declared return type string but returns int +error[main.php:3:5]: Undefined variable: $name +error[src/Helpers.php:8:1]: Function 'foo' declared return type string but returns int ``` The compiler also emits non-fatal warnings (unused variables, unreachable code). From 6ab4729529464a48e24614c141e6d3f7dad71fd0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Joaqu=C3=ADn=20Gamboa?= Date: Wed, 21 Oct 2026 20:43:37 -0300 Subject: [PATCH 4/6] docs: keep the autoload prelude probe note in its own pipeline entry The probe description now sits in its own bullet after func-args instead of extending the resolve/prelude entry, whose closing sentence main has since rewritten. --- docs/compiling/compilation-pipeline.md | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/docs/compiling/compilation-pipeline.md b/docs/compiling/compilation-pipeline.md index 406d5a02f8..b19b06f7f7 100644 --- a/docs/compiling/compilation-pipeline.md +++ b/docs/compiling/compilation-pipeline.md @@ -86,19 +86,20 @@ Physical source (.php or .lfc) when referenced. The web runtime prelude is injected with `--web`, and namespace/`use` rules rewrite references to fully-qualified - names. Autoloading is wired in around these steps. Because the autoload pass - runs after name resolution, a prelude needed only by an autoloaded file would - be missed; when autoload rules exist, a probe (`src/autoload/prelude_probe.rs`) - first runs name resolution and the autoload pass on a copy of the program, - asks each prelude's detector about the result, and rewinds every counter it - advanced, so the real pass injects those preludes and numbers its files, - classes and temporaries exactly as it would without the probe. Every included - and autoloaded file gets its own source identity, which is how diagnostics - name the file they come from. + names. Autoloading is wired in around these steps. - **func-args** — rewrites `func_num_args()`, `func_get_args()`, and `func_get_arg()` into a hidden variadic parameter plus ordinary PHP operations. This happens after autoloading but before manifest baking, optimization, and checking, so those later passes see the desugared callable shape. +- **autoload prelude probe** — the autoload pass runs after name resolution, so + a prelude needed only by an autoloaded file would be missed. When autoload + rules exist, a probe (`src/autoload/prelude_probe.rs`) first runs name + resolution and the autoload pass on a copy of the program, asks each + prelude's detector about the result, and rewinds every counter it advanced, + so the real pass injects those preludes and numbers its files, classes and + temporaries exactly as it would without the probe. Every included and + autoloaded file gets its own source identity, which is how diagnostics name + the file they come from. - **opcache-manifest-bake** — after autoload insertion and argument-introspection desugaring, replaces the placeholder OPcache manifest with the complete entry/include/autoload file set before constant folding and emits any preload From db958b57257fcd4781af81f63f1669f98650fd9b Mon Sep 17 00:00:00 2001 From: Vincenzo Petrucci Date: Sat, 24 Oct 2026 18:13:35 +0200 Subject: [PATCH 5/6] fix(autoload): keep probe global identities monotonic --- docs/compiling/compilation-pipeline.md | 8 +- docs/internals/the-type-checker.md | 2 + src/autoload/prelude_probe.rs | 12 +-- src/autoload/prelude_probe_tests.rs | 101 +++++++++++++++++++++++++ src/resolver/engine_includes.rs | 3 +- src/resolver/mod.rs | 19 ----- src/span.rs | 35 ++++++--- 7 files changed, 141 insertions(+), 39 deletions(-) create mode 100644 src/autoload/prelude_probe_tests.rs diff --git a/docs/compiling/compilation-pipeline.md b/docs/compiling/compilation-pipeline.md index ff0820a0f0..ed2f8f0a46 100644 --- a/docs/compiling/compilation-pipeline.md +++ b/docs/compiling/compilation-pipeline.md @@ -100,9 +100,11 @@ Physical source (.php or .lfc) a prelude needed only by an autoloaded file would be missed. When autoload rules exist, a probe (`src/autoload/prelude_probe.rs`) first runs name resolution and the autoload pass on a copy of the program, asks each - prelude's detector about the result, and rewinds every counter it advanced, - so the real pass injects those preludes and numbers its files, classes and - temporaries exactly as it would without the probe. Every included and + prelude's detector about the result, and restores the thread-local source IDs + and anonymous-class counter. Process-global synthetic spans and include + temporaries stay monotonic, so discarding a probe cannot recycle identities + allocated by another compilation thread. The real pass injects the detected + preludes with fresh global identities. Every included and autoloaded file gets its own source identity, which is how diagnostics name the file they come from. - **opcache-manifest-bake** — after autoload insertion and argument-introspection diff --git a/docs/internals/the-type-checker.md b/docs/internals/the-type-checker.md index e420a7d72f..711c36ba44 100644 --- a/docs/internals/the-type-checker.md +++ b/docs/internals/the-type-checker.md @@ -738,6 +738,8 @@ Warnings are returned through `CheckResult` and printed by the CLI without faili Checker errors and warnings carry only a `Span`. Each file the resolver or the autoload pass lexes gets its own source identity, and `Span::register_source_path` records which physical file that identity came from; `print_error` / `print_warning` fall back to `Span::source_path` when a diagnostic was not tagged with a file, so every positioned diagnostic prints as `kind[file:line:col]`. Class schema errors (circular inheritance, extending a `final` class, duplicate or unknown parent classes) point at the class declaration. +The discarded autoload prelude probe restores only thread-local source IDs and anonymous-class numbering. Synthetic spans and hidden value-include temporaries use monotonic process-global counters, so a probe cannot recycle identities allocated by another compiler thread. Gaps left by discarded work do not affect PHP values or name resolution. + ### Lookup indexes Large programs (thousands of classes) made several per-member scans of the whole class table dominate checking time. The checker keeps lazily built indexes for them, each stored with the `classes.len()` it was built for, so registering a class invalidates it: diff --git a/src/autoload/prelude_probe.rs b/src/autoload/prelude_probe.rs index ab48ae0474..bb32948658 100644 --- a/src/autoload/prelude_probe.rs +++ b/src/autoload/prelude_probe.rs @@ -15,9 +15,9 @@ //! result with each prelude's own detector, and hands back the groups found. The real pass then //! injects them exactly as if the entry file had used them, so the outcome is the one //! `--with-` gives, without forcing the whole surface past reachability pruning. -//! - The copy's work is discarded, including every counter it advanced (source identities, -//! synthetic spans, anonymous-class numbers, the resolver's hidden temporaries), so the real -//! pass numbers its files, classes and temporaries exactly as it would without the probe. +//! - The copy's work is discarded, restoring thread-local source IDs and anonymous-class +//! numbers so the real parse keeps those identities. Process-global synthetic spans and +//! include temporaries remain monotonic, preserving identities other threads allocated. //! - A probe failure is not reported here: the real pass hits the same error and reports it. use std::collections::HashSet; @@ -28,6 +28,10 @@ use crate::span::Span; use super::Registry; +#[cfg(test)] +#[path = "prelude_probe_tests.rs"] +mod tests; + /// Every prelude group the probe can report, with the detector that decides it. const PROBED_GROUPS: &[(&str, fn(&[Stmt]) -> bool)] = &[ ("pdo", crate::pdo_prelude::program_uses_pdo), @@ -54,7 +58,6 @@ pub fn autoloaded_prelude_groups( return HashSet::new(); } let span_counters = Span::counters_checkpoint(); - let resolver_counters = crate::resolver::counters_checkpoint(); let anonymous_classes = crate::parser::anonymous_class_counter(); let closure = crate::name_resolver::resolve(program.clone()) .ok() @@ -62,7 +65,6 @@ pub fn autoloaded_prelude_groups( super::run_collecting_included_with_defines(resolved, base_dir, registry, defines).ok() }); Span::restore_counters(span_counters); - crate::resolver::restore_counters(resolver_counters); crate::parser::restore_anonymous_class_counter(anonymous_classes); let Some((closure, _)) = closure else { return HashSet::new(); diff --git a/src/autoload/prelude_probe_tests.rs b/src/autoload/prelude_probe_tests.rs new file mode 100644 index 0000000000..651fd8cbe0 --- /dev/null +++ b/src/autoload/prelude_probe_tests.rs @@ -0,0 +1,101 @@ +//! Purpose: +//! Protects the identity allocation boundary between discarded probes and real autoloads. +//! +//! Called from: +//! - `autoload::prelude_probe` unit tests. +//! +//! Key details: +//! - Source and anonymous-class IDs are restored, while global include temporaries stay fresh. +//! - The fixture removes its files even when an assertion fails. + +use std::fs; +use std::path::PathBuf; + +use crate::parser::ast::{Program, StmtKind}; +use crate::span::Span; + +use super::{autoloaded_prelude_groups, Registry}; + +/// Owns the small Composer project used to exercise a real probe and autoload pass. +struct ProbeProject(PathBuf); + +impl ProbeProject { + /// Creates an autoloaded class with a value include and an anonymous class. + fn new() -> Self { + let path = std::env::temp_dir().join(format!( + "elephc_probe_counter_{}_{:?}", std::process::id(), std::thread::current().id(), + )); + fs::create_dir(&path).unwrap(); + let project = Self(path); + fs::create_dir(project.0.join("src")).unwrap(); + fs::write(project.0.join("composer.json"), + r#"{"autoload":{"psr-4":{"App\\":"src/"}}}"#).unwrap(); + fs::write(project.0.join("src/value.php"), " Program { + crate::parser::parse(&crate::lexer::tokenize(source).unwrap()).unwrap() +} + +/// Reads the resolver's hidden include temporary from the flattened top-level statements. +fn include_temporary_id(program: &[crate::parser::ast::Stmt]) -> Option { + program.iter().find_map(|stmt| match &stmt.kind { + StmtKind::Assign { name, .. } => name.strip_prefix("__elephc_inc_")?.parse().ok(), + StmtKind::NamespaceBlock { body, .. } | StmtKind::Synthetic(body) => { + include_temporary_id(body) + } + _ => None, + }) +} + +/// A probe preserves real-file and anonymous-class naming without recycling its include temporary. +#[test] +fn probe_restores_local_identities_but_keeps_include_temporaries_monotonic() { + let project = ProbeProject::new(); + let defines = std::collections::HashSet::new(); + let seed = crate::resolver::resolve_collecting_includes_continuing( + parse(" previous_temporary + 1, + "the probe's discarded include temporary must not be reused"); + let expected_anonymous = format!("class@anonymous#{anonymous_class}"); + assert!(loaded.iter().any(|stmt| matches!(&stmt.kind, + StmtKind::ClassDecl { name, .. } if name.ends_with(&expected_anonymous))), + "the real parse must retain its anonymous-class name"); +} diff --git a/src/resolver/engine_includes.rs b/src/resolver/engine_includes.rs index a6e542f8cc..faf75b5491 100644 --- a/src/resolver/engine_includes.rs +++ b/src/resolver/engine_includes.rs @@ -25,7 +25,8 @@ use super::include_path::fold_include_path; use super::state::ResolveState; /// Process-global counter producing unique hidden temporary names for value-position includes. -pub(super) static VALUE_INCLUDE_COUNTER: AtomicUsize = AtomicUsize::new(0); +/// Discarded probes consume names too, so concurrent compilations never recycle live temporaries. +static VALUE_INCLUDE_COUNTER: AtomicUsize = AtomicUsize::new(0); /// Where the value produced by an expression-position include must be delivered. pub(super) enum IncludeValueCapture { diff --git a/src/resolver/mod.rs b/src/resolver/mod.rs index 787c821b93..d6d49d7976 100644 --- a/src/resolver/mod.rs +++ b/src/resolver/mod.rs @@ -120,25 +120,6 @@ pub fn resolve_collecting_includes_with_defines( }) } -/// The resolver's hidden-temporary counters at one moment; see [`counters_checkpoint`]. -#[derive(Debug, Clone, Copy)] -pub struct ResolverCounters([usize; 1]); - -/// Captures the counters that name the resolver's hidden temporaries, to hand back to -/// [`restore_counters`] after a resolution whose result is discarded (the autoload prelude -/// probe), so the real resolution names its temporaries exactly as without it. -pub fn counters_checkpoint() -> ResolverCounters { - use std::sync::atomic::Ordering::SeqCst; - ResolverCounters([engine_includes::VALUE_INCLUDE_COUNTER.load(SeqCst)]) -} - -/// Rewinds the resolver's hidden-temporary counters to a [`counters_checkpoint`]. -pub fn restore_counters(checkpoint: ResolverCounters) { - use std::sync::atomic::Ordering::SeqCst; - let [value_include] = checkpoint.0; - engine_includes::VALUE_INCLUDE_COUNTER.store(value_include, SeqCst); -} - /// Resolves the includes of a file loaded INTO an ongoing compile (an autoloaded class file). /// /// Unlike [`resolve_collecting_includes_with_defines`] it keeps the source identities the diff --git a/src/span.rs b/src/span.rs index 835c24485a..242858d061 100644 --- a/src/span.rs +++ b/src/span.rs @@ -108,12 +108,11 @@ std::thread_local! { std::cell::RefCell::new(std::collections::HashMap::new()); } -/// The span counters at one moment; see [`Span::counters_checkpoint`]. +/// The thread-local source-ID counters at one moment; see [`Span::counters_checkpoint`]. #[derive(Debug, Clone, Copy)] pub struct SpanCounters { next_source_id: u32, next_synthetic_source_id: u32, - next_synthetic_line: u32, } #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] @@ -203,25 +202,21 @@ impl Span { NEXT_SOURCE_ID.with(|next| next.set(1)); } - /// Captures every span counter (include and synthetic source identities, synthetic lines), - /// to hand back to [`Span::restore_counters`] after work whose spans are discarded. + /// Captures thread-local include and synthetic source IDs for discarded work. + /// + /// Process-global synthetic lines stay monotonic: another thread may allocate live spans + /// while an autoload prelude probe is in progress. pub fn counters_checkpoint() -> SpanCounters { SpanCounters { next_source_id: NEXT_SOURCE_ID.with(|next| next.get()), next_synthetic_source_id: NEXT_SYNTHETIC_SOURCE_ID.with(|next| next.get()), - next_synthetic_line: NEXT_SYNTHETIC_LINE.load(std::sync::atomic::Ordering::Relaxed), } } - /// Rewinds the span counters to a [`Span::counters_checkpoint`], so a discarded trial parse - /// (the autoload prelude probe) leaves the numbering the real pass will use. + /// Restores thread-local source IDs after a discarded parse, leaving global spans fresh. pub fn restore_counters(checkpoint: SpanCounters) { NEXT_SOURCE_ID.with(|next| next.set(checkpoint.next_source_id)); NEXT_SYNTHETIC_SOURCE_ID.with(|next| next.set(checkpoint.next_synthetic_source_id)); - NEXT_SYNTHETIC_LINE.store( - checkpoint.next_synthetic_line, - std::sync::atomic::Ordering::Relaxed, - ); } /// Returns a source identity for a synthetic parse that [`Span::reset_source_ids`] must not @@ -389,6 +384,24 @@ impl Span { mod tests { use super::*; + /// Restoring discarded source IDs must not recycle synthetic spans allocated on another thread. + #[test] + fn probe_source_restore_keeps_concurrent_synthetic_spans_monotonic() { + let checkpoint = Span::counters_checkpoint(); + let source_id = Span::fresh_source_id(); + let synthetic_source_id = Span::fresh_synthetic_source_id(); + let concurrent_span = std::thread::spawn(Span::synthetic).join().unwrap(); + + Span::restore_counters(checkpoint); + + assert_eq!(Span::fresh_source_id(), source_id); + assert_eq!(Span::fresh_synthetic_source_id(), synthetic_source_id); + assert!( + Span::synthetic().line > concurrent_span.line, + "restoring thread-local IDs must preserve another thread's synthetic span" + ); + } + /// Spans are embedded in every token and AST node and flow through the /// recursive-descent parser's stack frames; growing this struct directly /// deepens no recursion but fattens every frame, and 2 MiB test threads From d28b4e0ccc04d144e8d7fa3794b4e0b226f0338e Mon Sep 17 00:00:00 2001 From: Vincenzo Petrucci Date: Sat, 24 Oct 2026 18:13:39 +0200 Subject: [PATCH 6/6] fix(diagnostics): name entry files in early autoload warnings --- docs/compiling/output-and-diagnostics.md | 8 +++++--- src/pipeline.rs | 9 ++++----- tests/autoload_diagnostics_tests.rs | 18 ++++++++++++++++++ 3 files changed, 27 insertions(+), 8 deletions(-) diff --git a/docs/compiling/output-and-diagnostics.md b/docs/compiling/output-and-diagnostics.md index fa675d6e5d..aa828b66e0 100644 --- a/docs/compiling/output-and-diagnostics.md +++ b/docs/compiling/output-and-diagnostics.md @@ -124,9 +124,11 @@ warning[src/Helpers.php:14:9]: Unreachable code The file is the entry file as given on the command line, or the included or autoloaded file that holds the statement, so a diagnostic in a large autoloaded -tree points at the right file. A position whose source file is not known (a -compiler-synthesized node) prints `kind[line:col]`, and a diagnostic with no -source position at all (some whole-program checks) prints `kind: message`. +tree points at the right file. The entry path is registered before frontend and +autoload processing, so early autoload warnings name it too. A position whose +source file is not known (a compiler-synthesized node) prints `kind[line:col]`, +and a diagnostic with no source position at all (some whole-program checks) +prints `kind: message`. Profile `note[…]` lines keep their `note[line:col]` form. ### `--quiet` / `-q` diff --git a/src/pipeline.rs b/src/pipeline.rs index 0581f3e8e6..599db754b8 100644 --- a/src/pipeline.rs +++ b/src/pipeline.rs @@ -187,6 +187,10 @@ fn compile_inner(config: CliConfig, entry: EntryKind<'_>) { } let mut timings = CompileTimings::new(emit_timings); + // The entry file uses source ID 0. Register it before any frontend or autoload diagnostic, + // including warnings from rejected autoload closures. The resolver only resets IDs from 1 + // upwards and leaves the path registry intact. + Span::register_source_path(0, filename.to_string()); let parsed = frontend::read_and_parse( filename, source_mode, &defines, &mut timings, entry.generated_source(), @@ -226,11 +230,6 @@ fn compile_inner(config: CliConfig, entry: EntryKind<'_>) { crate::progress::phase("resolve"); let phase_started = Instant::now(); - // The entry file lexes with the default source identity 0, which the resolver never hands - // out again (it rewinds the include counter to 1, and that does not touch the path registry). - // Registered before the resolver so an include error in the entry file names it too - // (`error[main.php:2:1]: Required file not found`). - Span::register_source_path(0, filename.to_string()); // `resolve_collecting_includes` also hands back the canonical path of every file the // resolver statically inlined — group 2 of the OPcache script manifest. let (ast, opcache_included_files) = diff --git a/tests/autoload_diagnostics_tests.rs b/tests/autoload_diagnostics_tests.rs index bf7fb2f899..b85cd9b4c6 100644 --- a/tests/autoload_diagnostics_tests.rs +++ b/tests/autoload_diagnostics_tests.rs @@ -68,6 +68,24 @@ fn check(dir: &Path) -> Output { const COMPOSER: &str = r#"{"autoload":{"psr-4":{"App\\":"src/"}}}"#; +/// Autoload registration warnings are emitted before include resolution and still name the entry. +#[test] +fn early_autoload_warning_names_the_entry_file() { + let dir = project( + "elephc_early_autoload_warning", + &[("main.php", "