diff --git a/README.md b/README.md index f63a64141f..541413af40 100644 --- a/README.md +++ b/README.md @@ -657,18 +657,18 @@ 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 inside an included or autoloaded file also names that file, as in `error[/app/src/Broken.php:4:41]: Undefined class: App\Missing` (see [diagnostic locations](docs/compiling/output-and-diagnostics.md#diagnostic-locations)). +A diagnostic with no source position (some whole-program checks) prints `error: message`. See [diagnostic locations](docs/compiling/output-and-diagnostics.md#diagnostic-locations). ## Project structure 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 12aed8d9af..bd0a62cceb 100644 --- a/docs/compiling/compilation-pipeline.md +++ b/docs/compiling/compilation-pipeline.md @@ -99,6 +99,17 @@ Physical source (.php or .lfc) `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 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 desugaring, replaces the placeholder OPcache manifest with the complete entry/include/autoload file set before constant folding and emits any preload diff --git a/docs/compiling/output-and-diagnostics.md b/docs/compiling/output-and-diagnostics.md index d0353c8fb7..75777e4393 100644 --- a/docs/compiling/output-and-diagnostics.md +++ b/docs/compiling/output-and-diagnostics.md @@ -114,19 +114,22 @@ and `--quiet` keep the compact plain output without progress lines. ### Diagnostic locations -Errors and warnings print as `error[line:col]: message` and -`warning[line:col]: message`. A diagnostic raised inside a file other than the -entry file (an `include`/`require` target, a Composer `autoload.files` entry, or -a class file loaded through PSR-4 or an SPL autoload rule) names that file -before the position, as the path it was loaded from: +Errors and warnings name the physical file, line and column they come from: ```text -error[3:1]: Undefined variable: $x -error[/app/src/Broken.php:4:41]: Undefined class: App\Missing -warning[/app/lib/helpers.php:9:5]: Unused variable: $tmp +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 ``` -Lexer and parser errors name their file in every case, the entry file included. +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. 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/docs/internals/the-lexer.md b/docs/internals/the-lexer.md index 7b51a4bfe7..0b104bd7a9 100644 --- a/docs/internals/the-lexer.md +++ b/docs/internals/the-lexer.md @@ -60,7 +60,7 @@ The cursor automatically tracks line and column — when it sees a `\n`, it incr The entry file keeps identity 0. Every other physical file the compile loads takes the next identity from `Span::fresh_source_id()`: each include execution (`resolver::files::parse_file`), each Composer `autoload.files` entry and PSR-4 or SPL-rule class file (`autoload::load_autoloaded_file`), and the includes those class files pull in. Only the root include resolution (`resolver::resolve_collecting_includes_with_defines`) rewinds the counter with `Span::reset_source_ids`; an autoloaded file resolves its own includes through `resolve_collecting_includes_continuing`, which does not. Two files therefore never share an identity within one compile, so a statement at the same line and column of two class files is two distinct keys in span-keyed checker maps (`throw_access_sites`, `builtin_call_types`, `loop_storage_types`, the local binding decisions). Autoloaded files used to be lexed with identity 0, which handed one class's readonly write guard to a plain write at the same position in another class (issue #1445). -Each loader also records the file behind its identity with `Span::register_source_path`. A checker diagnostic carries only a span, and the pipeline tags lowering, export and `--emit-ir` refusals with the entry filename whatever file they come from, so the diagnostic printer (`errors::report`, `errors::report_warning`) asks `Span::source_path()` first and uses a tag only for a span with no registered source. An error inside an included or autoloaded file therefore prints as `error[path:line:col]`. The entry file is never registered, so its diagnostics keep the `error[line:col]` form. The registry is per thread, like the identity counter it names. +Each loader also records the file behind its identity with `Span::register_source_path`. A checker diagnostic carries only a span, and the pipeline tags lowering, export and `--emit-ir` refusals with the entry filename whatever file they come from, so the diagnostic printer (`errors::report`, `errors::report_warning`) asks `Span::source_path()` first and uses a tag only for a span with no registered source. An error inside an included or autoloaded file therefore prints as `error[path:line:col]`. The pipeline registers the entry file as identity 0 before frontend and autoload processing, so its diagnostics also name that file. The registry is per thread, like the identity counter it names. A literal `eval` fragment parsed at EIR lowering carries its own identity from `Span::fresh_synthetic_source_id()`: a counter separate from the include one that `Span::reset_source_ids` rewinds at the start of every compile, and starting above the packed identity range. A fragment span therefore cannot alias the root file's or an included file's coordinates in span-keyed maps such as `builtin_call_types`. diff --git a/docs/internals/the-type-checker.md b/docs/internals/the-type-checker.md index d2a5fbf0be..cde6650bab 100644 --- a/docs/internals/the-type-checker.md +++ b/docs/internals/the-type-checker.md @@ -736,6 +736,21 @@ 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` prefer `Span::source_path` over an explicit file tag, so an entry-file tag cannot rename a position in another file. The pipeline registers the entry path before frontend processing, making positioned diagnostics print 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: + +- `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 ae8e6cdc18..8159e5be82 100644 --- a/docs/php/types.md +++ b/docs/php/types.md @@ -533,11 +533,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). diff --git a/src/autoload/mod.rs b/src/autoload/mod.rs index 43a90a0c55..86eb947037 100644 --- a/src/autoload/mod.rs +++ b/src/autoload/mod.rs @@ -15,6 +15,7 @@ mod alias; mod dev_packages; +pub mod prelude_probe; mod index; mod interpret; mod registry; diff --git a/src/autoload/prelude_probe.rs b/src/autoload/prelude_probe.rs new file mode 100644 index 0000000000..bb32948658 --- /dev/null +++ b/src/autoload/prelude_probe.rs @@ -0,0 +1,77 @@ +//! 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, 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; +use std::path::Path; + +use crate::parser::ast::{Program, Stmt}; +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), + ("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 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::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/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/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 b90ccc0acf..8005ca32d8 100644 --- a/src/errors/report.rs +++ b/src/errors/report.rs @@ -11,7 +11,7 @@ //! the same switch the live spinner and `--timings` report use, so plain/piped/`--quiet` //! runs keep byte-identical text to before. //! - A diagnostic whose span was lexed from an included or autoloaded file names that file -//! (`Span::source_path`), ahead of any entry-filename tag; entry-file positions keep their form. +//! (`Span::source_path`), ahead of any entry-filename tag. The pipeline also registers the entry. use super::{CompileError, CompileWarning}; @@ -20,8 +20,7 @@ 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` (a position in the entry file, or a -/// node with no registered source) +/// - With line and column only: `error[line:col]: message` (a node with no registered source) /// - With no location metadata: `error: message` /// /// An error whose span was lexed from an included or autoloaded file names that file @@ -49,7 +48,7 @@ pub fn print_error(error: &CompileError) { /// Prints a compiler warning to stderr with optional line and column. /// /// Format varies based on available metadata: -/// - In an included or autoloaded file: `warning[file:line:col]: message` +/// - With a registered source file: `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) { @@ -74,7 +73,7 @@ pub fn print_warning(warning: &CompileWarning) { /// every lowering, export and `--emit-ir` refusal with the ENTRY filename: preferring that tag /// printed an included file's line and column under the entry file's name. A tag still names /// the file when the span has no position or no registered source, which covers every -/// entry-file span (identity 0 is never registered) and keeps those diagnostics as they were. +/// standalone span outside the pipeline. The pipeline registers the entry as identity 0. fn diagnostic_file(file: &Option, span: crate::span::Span) -> Option { if span.line != 0 { if let Some(path) = span.source_path() { 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 e4081dfa51..b543d982f6 100644 --- a/src/parser/mod.rs +++ b/src/parser/mod.rs @@ -91,6 +91,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)); +} + /// Returns whether `name` is a synthetic anonymous-class name from `next_anonymous_class_name`. pub(crate) fn is_anonymous_class_name(name: &str) -> bool { name.trim_start_matches('\\').starts_with("class@anonymous#") diff --git a/src/pipeline.rs b/src/pipeline.rs index f676202a44..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(), @@ -237,7 +241,7 @@ fn compile_inner(config: CliConfig, entry: EntryKind<'_>) { process::exit(1); } }; - let ast = match autoload::collect_aliases(ast) { + let ast =match autoload::collect_aliases(ast) { Ok(ast) => ast, Err(error) => { crate::progress::clear(); @@ -265,6 +269,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 +330,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 +355,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 +373,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 +394,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 +490,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 +503,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 +529,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 +551,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..faf75b5491 100644 --- a/src/resolver/engine_includes.rs +++ b/src/resolver/engine_includes.rs @@ -25,6 +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. +/// 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. diff --git a/src/span.rs b/src/span.rs index a6a85996ac..53bd781b4b 100644 --- a/src/span.rs +++ b/src/span.rs @@ -14,9 +14,6 @@ //! preserving the AST and parser-frame size while distinguishing equal source coordinates. //! A pair too wide for those bits (a column past 65535, or a very late source identity) is //! interned in a process-wide table instead, so no source is ever too wide to compile. -//! - Every physical file a compile loads besides the entry (each include execution, each -//! autoloaded class file) gets its own identity from one counter that only the ROOT include -//! resolution rewinds, and its path is registered for diagnostics (`Span::source_path`). /// The first line number handed out to synthetically built nodes. /// @@ -101,18 +98,25 @@ std::thread_local! { /// file parses in one compile, which is pathological). static NEXT_SYNTHETIC_SOURCE_ID: std::cell::Cell = const { std::cell::Cell::new(SOURCE_ID_MASK + 1) }; - /// The physical file each included or autoloaded source identity was lexed from. + /// The physical file each source identity was lexed from, for diagnostics. + /// Per thread like `NEXT_SOURCE_ID`, so parallel compilations name their own files. + /// The pipeline registers the entry file under identity 0 before frontend processing. /// - /// A checker or lowering diagnostic carries only a span, so without this an error inside an - /// autoloaded class file printed a bare `error[line:col]` that read as a position in the entry - /// file. Per thread like [`NEXT_SOURCE_ID`], whose identities it names: two compiles running - /// on two threads of one process each see their own files. An entry is overwritten when its - /// identity is handed out again after [`Span::reset_source_ids`], so a lookup names the latest - /// file lexed with that identity. The entry file (identity 0) is never registered. + /// 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 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, +} + #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] /// Source position span for AST nodes. pub struct Span { @@ -170,11 +174,7 @@ impl Span { } } - /// Returns a fresh source identity for a separately parsed included or autoloaded file. - /// - /// Unique within one compile: only the root include resolution rewinds the counter - /// ([`Span::reset_source_ids`]), so an autoloaded class file and the includes it pulls in - /// never reuse the identity of a file the entry's resolution already handed out. + /// Returns a fresh source identity for a separately parsed included file. pub fn fresh_source_id() -> u32 { NEXT_SOURCE_ID.with(|next| { let id = next.get(); @@ -185,17 +185,7 @@ impl Span { }) } - /// Resets source identities at the beginning of a compile's root include resolution. - /// - /// Called once per compile, before the entry file's includes are parsed. Autoloaded files are - /// loaded later in the same compile and must NOT reset again: a reset there handed their - /// includes the identities of the entry's includes, so span-keyed checker results (a readonly - /// write guard, a builtin call type, a binding decision) leaked between files (issue #1445). - pub fn reset_source_ids() { - NEXT_SOURCE_ID.with(|next| next.set(1)); - } - - /// Records the physical file an included or autoloaded source identity was lexed from. + /// 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| { @@ -203,21 +193,43 @@ impl Span { }); } - /// Returns the included or autoloaded file this span was lexed from, when one was registered. - /// - /// `None` for the entry file (identity 0), a synthetic `eval` fragment, and any identity no - /// loader registered; callers then fall back to the bare `line:col` form. + /// 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 compile's root include resolution. + /// + /// Only the root resolver resets the counter. Autoloaded files and their includes continue + /// allocating fresh identities, so span-keyed checker results cannot leak between files. + pub fn reset_source_ids() { + NEXT_SOURCE_ID.with(|next| next.set(1)); + } + + /// 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()), + } + } + + /// 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)); + } + /// Returns a source identity for a synthetic parse that [`Span::reset_source_ids`] must not /// rewind. /// /// Used for literal `eval` fragments: an included file's identity is only unique WITHIN one - /// compile, so a fragment sharing that counter could alias an already-stored include id in - /// span-keyed maps such as `builtin_call_types` (issue #1291). + /// include-resolution unit, so a fragment sharing that counter could alias an already-stored + /// include id in span-keyed maps such as `builtin_call_types` (issue #1291). pub fn fresh_synthetic_source_id() -> u32 { NEXT_SYNTHETIC_SOURCE_ID.with(|next| { let id = next.get(); @@ -377,6 +389,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 @@ -399,8 +429,8 @@ mod tests { } /// A registered identity names its file through every span lexed with it, an unregistered - /// one and the entry file's identity 0 name none, and a later registration of the same - /// identity (after a reset hands it out again) replaces the earlier file. + /// one and an entry identity not yet registered by the pipeline name none. Registering the + /// same identity after a reset replaces the earlier file. #[test] fn registered_source_paths_name_the_file_of_their_spans() { Span::register_source_path(9_101, "src/Counter.lfc"); @@ -410,7 +440,7 @@ mod tests { ); assert_eq!(span.source_path().as_deref(), Some("src/Counter.lfc")); assert_eq!(Span::new_in_source(9, 9, 9_102).source_path(), None); - assert_eq!(Span::new(9, 9).source_path(), None, "the entry file is never registered"); + assert_eq!(Span::new(9, 9).source_path(), None, "source 0 is unregistered outside the pipeline"); Span::register_source_path(9_101, "src/Fixed.lfc"); assert_eq!(span.source_path().as_deref(), Some("src/Fixed.lfc")); } 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 dbeb482aba..d408ff9fd3 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(), @@ -133,6 +136,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 86df2482b5..823371a31a 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; @@ -617,6 +617,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 f73be1415a..beaa689158 100644 --- a/src/types/checker/inference/ops.rs +++ b/src/types/checker/inference/ops.rs @@ -532,8 +532,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 eb630b7a51..127fe7b8ca 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). @@ -443,6 +462,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..b85cd9b4c6 --- /dev/null +++ b/tests/autoload_diagnostics_tests.rs @@ -0,0 +1,208 @@ +//! 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/"}}}"#; + +/// 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", "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", "/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", "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"); +}