diff --git a/CLAUDE.md b/CLAUDE.md index 555d2c4dc..347eea8ed 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -94,31 +94,28 @@ p compile --mode pex # Run model checking with PEx backend p check --mode pex -# Multi-error compilation — report ALL type errors in one pass -# instead of aborting on the first. Default is strict (one error, -# exit 1). Errors are sorted by source location. -P_COMPILER_COLLECT_ERRORS=1 p compile +# By default, `p compile` reports ALL type errors in one pass. +# Use --strict-errors / -se to restore the legacy abort-on-first behavior. +p compile --strict-errors ``` ## Multi-Error Type Checking (Compiler) -The C# compiler under `Src/PCompiler/CompilerCore/` supports a **collecting -mode** that gathers diagnostics through `IDiagnosticCollector` instead of -throwing on the first error. Opt in via the `P_COMPILER_COLLECT_ERRORS=1` -environment variable. Default (strict) mode is bit-for-bit identical to -pre-3.0 behavior for valid programs and same exit code on invalid programs. +The C# compiler under `Src/PCompiler/CompilerCore/` reports all type errors +in one pass by default (P 3.0+). Diagnostics flow through +`IDiagnosticCollector` and are flushed (sorted by source location) at end of +compilation. Users opt out via `--strict-errors` (`-se`), which restores the +legacy abort-on-first behavior. ### Architecture - **`IDiagnosticCollector` / `DefaultDiagnosticCollector`** — strict mode - rethrows immediately; collecting mode appends to a list, returns, and the - collector is flushed at end of compilation via - `Compiler.FlushCollectedDiagnostics`. + rethrows immediately; collecting mode (default) appends to a list. The + collector is flushed via `Compiler.FlushCollectedDiagnostics` at end of + compilation. - **`ErrorType` (singleton sentinel) / `ErrorExpr`** — substituted for - failed-to-type-check expressions in collecting mode. `ErrorType` claims - compatibility with every other type so downstream compatibility checks - cascade-suppress (one root-cause error doesn't generate a chain of - "incompatible operand" diagnostics). + failed-to-type-check expressions. `ErrorType` claims compatibility with + every other type so downstream compatibility checks cascade-suppress. - **`PLanguageType.IsSameTypeAs`** has a symmetric short-circuit when either side is `ErrorType`. - **`TypeCheckingUtils.CheckAssignable`** is the cascade-aware compatibility @@ -134,7 +131,8 @@ Follow the convention in `ExprVisitor.cs`'s class doc: 1. **Visit children first** — so their internal errors surface even if the parent lookup fails. -2. **Short-circuit on `ErrorType`** — `if (subExpr.Type is ErrorType) return new ErrorExpr(context);` +2. **Short-circuit on `ErrorType`** — + `if (subExpr.Type is ErrorType) return new ErrorExpr(context);` at the top of each method after visiting children. 3. **Convert each `throw handler.X(...)` to**: ```csharp @@ -161,7 +159,7 @@ This way, one bad item doesn't abort the pass for siblings in collecting mode. T ### Test fixtures -- **`Tst/UnitTests/TypeChecker/DiagnosticCollectorTest.cs`** — collector contract + env-var parsing +- **`Tst/UnitTests/TypeChecker/DiagnosticCollectorTest.cs`** — collector contract + default mode verification - **`Tst/UnitTests/TypeChecker/Phase1DormancyTest.cs`** — iterates every Correct/StaticError dir; asserts both modes succeed on Correct/ and that collecting count ≥ strict count on StaticError/ - **`Tst/UnitTests/TypeChecker/MultiErrorAcceptanceTest.cs`** — `[TestCaseSource]` with pinned strict/collecting counts on curated multi-error files under `Tst/RegressionTests/Feature3Exprs/StaticError/` diff --git a/ChangeList.md b/ChangeList.md index b475eaea7..6f91bd40d 100644 --- a/ChangeList.md +++ b/ChangeList.md @@ -1,18 +1,20 @@ P 3.0 Changes -=== Multi-Error Type Checker (#957, #963, #965) === -- Opt-in via `P_COMPILER_COLLECT_ERRORS=1` environment variable. -- When enabled, `p compile` reports all type errors in one pass instead of - aborting on the first error. Errors are sorted by source location. -- Cascade-suppression rules (ErrorType/ErrorExpr sentinels + - `TypeCheckingUtils.CheckAssignable`) prevent one root-cause error from +=== Multi-Error Type Checker (#957, #963, #965, #967, #970) === +- `p compile` now reports ALL type errors in one pass by default, sorted by + source location, instead of aborting on the first error. +- Cascade-suppression (ErrorType/ErrorExpr sentinels + + `TypeCheckingUtils.CheckAssignable`) prevents one root-cause error from generating downstream "incompatible operand" noise. -- Cross-machine `` lookup (in `x is ` test expressions) now - resolves against the instance's specific machine; same-named states in - unrelated machines no longer bind silently (#963). -- Default behavior (env var unset) is unchanged — strict mode still - aborts on the first error, bit-for-bit identical to pre-3.0 behavior - for valid programs and same exit code on invalid programs. +- Pass-level tolerance in `Analyzer.cs` (`TolerantStep`) so one bad + machine/function doesn't clobber diagnostics from its siblings (#967). +- Cross-machine `` lookup in `x is ` test expressions now + resolves against the instance's specific machine (#963). +- New CLI flag `--strict-errors` / `-se` opts back into legacy abort-on- + first behavior for users / CI scripts that depend on it (#970). +- Exit codes unchanged: 0 on success, 1 on any error. The change is + user-visible only in the number of diagnostics emitted per failed + compile. === First PL === - branch: `dev_p3.0/cleanup_targets` diff --git a/README.md b/README.md index 0a1f9b05a..b93e1d41f 100644 --- a/README.md +++ b/README.md @@ -59,22 +59,30 @@ Validate that production systems conform to their formal P specifications. 👉 [Learn about PObserve](https://p-org.github.io/P/advanced/pobserve/pobserve/) -### Multi-Error Compilation +### Multi-Error Compilation (P 3.0+) -Set `P_COMPILER_COLLECT_ERRORS=1` to make `p compile` report ALL type errors -in one pass instead of aborting on the first. Errors are sorted by source -location with cascade-suppression so root causes surface without downstream -noise. +`p compile` reports ALL type errors in one pass by default, sorted by source +location. Cascade-suppression keeps root causes surfacing without downstream +noise: -```bash -$ P_COMPILER_COLLECT_ERRORS=1 p compile +``` +$ p compile [Error:] [bad.p:6:4] got type: bool, expected: int [Error:] [bad.p:8:13] could not find variable 'undeclaredVar' [Error:] [bad.p:9:16] incompatible binary operands int and string ``` -Particularly useful with AI fix loops (PeasyAI, Cursor) and large refactors — -fix N errors per LLM round-trip instead of N round-trips per N errors. +Use `--strict-errors` (or `-se`) to restore the legacy abort-on-first +behavior: + +``` +$ p compile --strict-errors +[Error:] [bad.p:6:4] got type: bool, expected: int +``` + +The new default is particularly useful with AI fix loops (PeasyAI, Cursor) +and large refactors — fix N errors per LLM round-trip instead of N +round-trips per N errors. ## The P Framework diff --git a/Src/PCompiler/CompilerCore/CompilerConfiguration.cs b/Src/PCompiler/CompilerCore/CompilerConfiguration.cs index d1e432d40..0a34aaf33 100644 --- a/Src/PCompiler/CompilerCore/CompilerConfiguration.cs +++ b/Src/PCompiler/CompilerCore/CompilerConfiguration.cs @@ -37,7 +37,11 @@ public CompilerConfiguration() PObservePackageName = $"{ProjectName}.pobserve"; ProjectRootPath = new DirectoryInfo(Directory.GetCurrentDirectory()); LocationResolver = new DefaultLocationResolver(); - ContinueOnError = ReadContinueOnErrorEnvVar(); + // Collecting mode is the default — report all type errors in one + // pass instead of aborting on the first. Users opt OUT via the + // `--strict-errors` (`-se`) CLI flag, which the PCompilerOptions + // parser flips to false. + ContinueOnError = true; Diagnostics = new DefaultDiagnosticCollector(ContinueOnError); Handler = new DefaultTranslationErrorHandler(LocationResolver, Diagnostics); OutputLanguages = new List{CompilerOutput.PChecker}; @@ -49,19 +53,6 @@ public CompilerConfiguration() TargetProofBlocks = new List(); Parallelism = Math.Max(Environment.ProcessorCount / 2, 1); } - - /// - /// Reads the P_COMPILER_COLLECT_ERRORS environment variable. - /// Any non-empty value other than "0"/"false" (case-insensitive) - /// enables collecting mode. Defaults to false (strict, throw-on-first). - /// - private static bool ReadContinueOnErrorEnvVar() - { - var raw = Environment.GetEnvironmentVariable("P_COMPILER_COLLECT_ERRORS"); - if (string.IsNullOrWhiteSpace(raw)) return false; - return !raw.Equals("0", StringComparison.OrdinalIgnoreCase) - && !raw.Equals("false", StringComparison.OrdinalIgnoreCase); - } /// /// Initializes a new instance of the class with specific settings. /// @@ -102,7 +93,8 @@ public CompilerConfiguration(ICompilerOutput output, DirectoryInfo outputDir, IL PObservePackageName = pObservePackageName ?? $"{ProjectName}.pobserve"; ProjectRootPath = projectRoot; LocationResolver = new DefaultLocationResolver(); - ContinueOnError = ReadContinueOnErrorEnvVar(); + // See parameterless ctor for collecting-mode rationale. + ContinueOnError = true; Diagnostics = new DefaultDiagnosticCollector(ContinueOnError); Handler = new DefaultTranslationErrorHandler(LocationResolver, Diagnostics); OutputLanguages = outputLanguages; @@ -112,8 +104,9 @@ public CompilerConfiguration(ICompilerOutput output, DirectoryInfo outputDir, IL } /// - /// See . Wired from - /// P_COMPILER_COLLECT_ERRORS at construction. Phase 1 wiring only. + /// See . Defaults + /// to true (collecting mode). Flipped to false by the CLI flag + /// --strict-errors / -se in PCompilerOptions. /// public bool ContinueOnError { get; set; } diff --git a/Src/PCompiler/CompilerCore/ICompilerConfiguration.cs b/Src/PCompiler/CompilerCore/ICompilerConfiguration.cs index 396178fa0..4c8f75287 100644 --- a/Src/PCompiler/CompilerCore/ICompilerConfiguration.cs +++ b/Src/PCompiler/CompilerCore/ICompilerConfiguration.cs @@ -26,13 +26,12 @@ public interface ICompilerConfiguration int Parallelism { get; } /// - /// When true, the type checker collects diagnostics and continues - /// instead of throwing on the first error. See - /// for the contract. Driven by env var P_COMPILER_COLLECT_ERRORS - /// (any non-empty / non-"0" value enables it). - /// - /// Phase 1: scaffolding only. No visitor currently reports through the - /// collector, so flipping this flag has no observable effect yet. + /// When true (the default), the type checker collects diagnostics and + /// continues instead of throwing on the first error. See + /// for the contract. + /// Users opt OUT of collecting via the CLI flag + /// --strict-errors / -se, which restores the legacy + /// abort-on-first behavior. /// bool ContinueOnError { get; } diff --git a/Src/PCompiler/PCommandLine/Options/PCompilerOptions.cs b/Src/PCompiler/PCommandLine/Options/PCompilerOptions.cs index 5fc51c400..0af92260d 100644 --- a/Src/PCompiler/PCommandLine/Options/PCompilerOptions.cs +++ b/Src/PCompiler/PCommandLine/Options/PCompilerOptions.cs @@ -75,6 +75,11 @@ internal PCompilerOptions() Parser.AddArgument("pobserve-package", "po", "PObserve package name").IsHidden = true; Parser.AddArgument("debug", "d", "Enable debug logs", typeof(bool)).IsHidden = true; + + Parser.AddArgument("strict-errors", "se", + "Abort on the first type error (legacy P 2.x behavior). " + + "By default, the compiler reports all type errors in one pass.", + typeof(bool)); var pvGroup = Parser.GetOrCreateGroup("pverifier", "PVerifier options"); pvGroup.AddArgument("timeout", "t", "Set SMT solver timeout in seconds", typeof(int)).IsHidden = true; @@ -168,6 +173,17 @@ private static void UpdateConfigurationWithParsedArgument(CompilerConfiguration case "debug": compilerConfiguration.Debug = true; break; + case "strict-errors": + // Opt out of collecting mode. Replaces the parameterless + // ctor's default (true) — and reconstructs the collector + + // handler so the strict-mode collector throws on first + // Report rather than appending. Order matters: collector + // first, then handler (which holds a reference to it). + compilerConfiguration.ContinueOnError = false; + compilerConfiguration.Diagnostics = new DefaultDiagnosticCollector(continueOnError: false); + compilerConfiguration.Handler = new DefaultTranslationErrorHandler( + compilerConfiguration.LocationResolver, compilerConfiguration.Diagnostics); + break; case "timeout": compilerConfiguration.Timeout = (int)option.Value; break; diff --git a/Src/PeasyAI/src/ui/mcp/tools/compilation.py b/Src/PeasyAI/src/ui/mcp/tools/compilation.py index 579d58033..8d757f267 100644 --- a/Src/PeasyAI/src/ui/mcp/tools/compilation.py +++ b/Src/PeasyAI/src/ui/mcp/tools/compilation.py @@ -36,7 +36,7 @@ def register_compilation_tools(mcp, get_services, with_metadata): @mcp.tool( name="peasy-ai-compile", - description="Compile a P project and return compilation results. The project directory must contain a .pproj file. On failure, the response includes parsed errors with file, line, and message details. Use peasy-ai-fix-compile-error or peasy-ai-fix-all to resolve compilation errors. Tip: set P_COMPILER_COLLECT_ERRORS=1 in the project environment to receive ALL type errors in a single response (compiler 3.0+) — lets fix-all converge in fewer iterations." + description="Compile a P project and return compilation results. The project directory must contain a .pproj file. On failure, the response includes ALL parsed errors with file, line, and message details (P compiler 3.0+ defaults to multi-error mode). Use peasy-ai-fix-compile-error or peasy-ai-fix-all to resolve compilation errors." ) def p_compile(params: PCompileParams) -> Dict[str, Any]: logger.info(f"[TOOL] peasy-ai-compile: {params.path}") diff --git a/Src/PeasyAI/src/ui/mcp/tools/fixing.py b/Src/PeasyAI/src/ui/mcp/tools/fixing.py index 67337ade3..5fa7ad701 100644 --- a/Src/PeasyAI/src/ui/mcp/tools/fixing.py +++ b/Src/PeasyAI/src/ui/mcp/tools/fixing.py @@ -410,7 +410,7 @@ def fix_checker_error(params: FixCheckerErrorParams) -> Dict[str, Any]: @mcp.tool( name="peasy-ai-fix-all", - description="Iteratively compile, detect errors, and fix them in a loop until the project compiles successfully or max_iterations is reached. This is the recommended way to fix multiple compilation errors at once — it automatically re-compiles after each fix to catch cascading issues. Use this instead of calling peasy-ai-fix-compile-error repeatedly. Tip: with P compiler 3.0+, set P_COMPILER_COLLECT_ERRORS=1 to get all type errors per compile in one batch — the loop converges in N/k iterations instead of N, where k is the average errors-per-iteration." + description="Iteratively compile, detect errors, and fix them in a loop until the project compiles successfully or max_iterations is reached. This is the recommended way to fix multiple compilation errors at once — it automatically re-compiles after each fix to catch cascading issues. With P compiler 3.0+ (multi-error mode is default), each compile reports all current errors so the loop converges in O(N/k) iterations instead of O(N). Use this instead of calling peasy-ai-fix-compile-error repeatedly." ) def fix_iteratively(params: FixIterativelyParams) -> Dict[str, Any]: logger.info(f"[TOOL] peasy-ai-fix-all: {params.project_path}") diff --git a/Tst/RegressionTests/Feature3Exprs/StaticError/MultipleErrors/MultipleErrors.p b/Tst/RegressionTests/Feature3Exprs/StaticError/MultipleErrors/MultipleErrors.p index 75664e4c3..0365adb0b 100644 --- a/Tst/RegressionTests/Feature3Exprs/StaticError/MultipleErrors/MultipleErrors.p +++ b/Tst/RegressionTests/Feature3Exprs/StaticError/MultipleErrors/MultipleErrors.p @@ -22,7 +22,7 @@ // - Strict mode (default): exit 1 after the first error is reported. // Identical to the historical behavior — this file is no different // from any other single-error StaticError test in this mode. -// - Collecting mode (P_COMPILER_COLLECT_ERRORS=1): exit 1 after +// - Default (collecting): exit 1 after // reporting all 4 independent errors. The Phase1DormancyTest // fixture asserts `collecting_count >= strict_count`, which holds // trivially with strict=1; the stronger acceptance check lives in diff --git a/Tst/UnitTests/TypeChecker/DiagnosticCollectorTest.cs b/Tst/UnitTests/TypeChecker/DiagnosticCollectorTest.cs index 885defc5a..17f973127 100644 --- a/Tst/UnitTests/TypeChecker/DiagnosticCollectorTest.cs +++ b/Tst/UnitTests/TypeChecker/DiagnosticCollectorTest.cs @@ -170,46 +170,21 @@ public void Diagnostics_IsLiveView_NotSnapshot() Assert.AreEqual("late arrival", liveView[0].Message); } - // Env var parsing — see CompilerConfiguration.ReadContinueOnErrorEnvVar. - // Tested via the public ContinueOnError property after construction so we - // don't have to mark the private helper internal. Each case saves and - // restores the var to keep test isolation. - // [NonParallelizable] because this test mutates a process-global env var - // (Environment.SetEnvironmentVariable). Any parallel test that constructs - // a CompilerConfiguration would read the temporarily-set value and run - // in the wrong mode — silently masking real strict-mode regressions. - // NUnit runs tests in different fixtures in parallel by default; this - // attribute forces sequential execution for THIS test. - [TestCase("1", true)] - [TestCase("true", true)] - [TestCase("True", true)] - [TestCase("TRUE", true)] - [TestCase("yes", true)] // any non-"0"/"false" non-empty value wins - [TestCase("anything-else", true)] - [TestCase("0", false)] - [TestCase("false", false)] - [TestCase("False", false)] - [TestCase("FALSE", false)] - [TestCase("", false)] // empty string treated as unset - [TestCase(" ", false)] // whitespace-only treated as unset - [TestCase(null, false)] // unset - [NonParallelizable] - public void ContinueOnError_ReadsEnvVar(string envValue, bool expected) + [Test] + public void CompilerConfiguration_DefaultsToCollectingMode() { - const string envName = "P_COMPILER_COLLECT_ERRORS"; - var saved = Environment.GetEnvironmentVariable(envName); - try - { - Environment.SetEnvironmentVariable(envName, envValue); - var config = new CompilerConfiguration(); - Assert.AreEqual(expected, config.ContinueOnError, - $"P_COMPILER_COLLECT_ERRORS={envValue ?? "(unset)"}"); - // The collector's mode must agree with the config's flag. - Assert.AreEqual(expected, config.Diagnostics.ContinueOnError); - } - finally - { - Environment.SetEnvironmentVariable(envName, saved); - } + // P 3.0+ contract: collecting mode is the default. Users opt out via + // the `--strict-errors` CLI flag, which PCompilerOptions flips after + // construction. If this test fails, the default has regressed — check + // the parameterless CompilerConfiguration constructor. + var config = new CompilerConfiguration(); + Assert.IsTrue(config.ContinueOnError, + "CompilerConfiguration() must default ContinueOnError to true (collecting mode)"); + Assert.IsNotNull(config.Diagnostics); + Assert.IsTrue(config.Diagnostics.ContinueOnError, + "The collector's mode must agree with the config's flag at construction"); + // Handler.Diagnostics must be the same instance as config.Diagnostics + // (invariant guarded by DefaultTranslationErrorHandler's null-throw). + Assert.AreSame(config.Diagnostics, config.Handler.Diagnostics); } }