Simon Jakobi pushed to branch wip/sjakobi/T27602 at Glasgow Haskell Compiler / GHC Commits: db959f83 by Simon Jakobi at 2026-08-14T15:16:44-04:00 testsuite: Expect length001 failure in nonmoving_thr_sanity length001 relies on an optimization rule to avoid excessive stack use. The nonmoving_thr_sanity way does not enable optimization, so classify its stack overflow as an expected failure, as is already done for the other unoptimized nonmoving ways. Assisted-by: gpt-5.6-sol via Codex CLI - - - - - 4f2b7d90 by Simon Jakobi at 2026-08-14T15:16:44-04:00 testsuite: Omit T22859 in nonmoving threaded ways T22859 checks allocation-limit handlers with output that depends on precise allocation behaviour. The nonmoving threaded ways change where these limits are reached, just as the already-omitted LLVM ways do. Omit these ways instead of treating their incidental output differences as test failures. Assisted-by: gpt-5.6-sol via Codex CLI - - - - - c4abddcb by Simon Jakobi at 2026-08-14T15:16:44-04:00 rts/js: Implement eq_thread, and test Eq/Ord ThreadId (#16761) Since d1f3c63701, Eq ThreadId is implemented via the RTS function eq_thread, but the JS RTS never provided it, so comparing ThreadIds for equality on the JS backend crashed with ReferenceError: h$eq_thread is not defined Like the C implementation, h$eq_thread uses pointer equality: The JS RTS has exactly one thread object per thread. Since previously no test exercised eq_thread directly, this commit adds a test covering equality, its stability across GC, and agreement with Ord. Assisted-by: Claude Fable 5 - - - - - 4a7defa1 by Simon Jakobi at 2026-08-14T15:16:44-04:00 testsuite: Make listThreads1 insensitive to the RTS's own threads listThreads1 expected `listThreads` to return exactly [ThreadId 1]. That holds only under a non-threaded RTS. Under a threaded RTS however there are more threads present, so we change the test to simply check that `myThreadId` is present in the list. Assisted-by: Claude Opus 5 - - - - - b757727a by Vladislav Zavialov at 2026-08-14T15:17:27-04:00 Fix tcLookupId panic with RequiredTypeArguments and PatternSynonyms (#27586) The arguments declared on the left-hand side of a pattern synonym are looked up as term variables bound by its right-hand side. Prior to this patch, that lookup panicked with RequiredTypeArguments: data T a where MkT :: forall a -> T a pattern P :: Int -> T Int pattern P x = MkT x On the RHS, `x` looks like a term argument, so the renamer binds it in the term namespace. Only during type checking does it turn out to be a type variable, so the lookup on the LHS finds an ATyVar rather than an ATcId. As the lookup was done with tcLookupId, it resulted in a panic. Now the arguments are looked up with tcLookupPatSynArg, which reports an illegal term-level use of `x`, just as an ordinary function definition `f (MkT x) = x` does. Test cases: T27586a T27586b T27586c Assisted-by: Claude Opus 5 - - - - - 30e97b01 by Simon Jakobi at 2026-08-15T17:08:49+02:00 testsuite: Show baseline sample count and spread in perf failures A perf baseline is the mean of all samples recorded for a commit, so a single outlier can silently corrupt it. Previously, the failure output gave no hint about such outliers: the baseline printed as one number. In #27602, T27336's peak_megabytes_allocated baseline showed as 757 when the underlying samples were 605 and 909. When the baseline is averaged from more than one sample, say so in the failure message and list the samples, both in the one-line stat-failure reason and in the detail block. Single-sample baselines print exactly as before. Context: #27602 Assisted-by: Claude Fable 5 - - - - - dab95c58 by Simon Jakobi at 2026-08-15T17:08:49+02:00 ci: Clarify comment on pushing perf notes after failures Context: #27602 Assisted-by: Claude Fable 5 - - - - - 748b9ae2 by Simon Jakobi at 2026-08-15T17:09:27+02:00 testsuite: Don't format baseline samples with '%g' '%g' renders large byte counts (e.g. max_bytes_used samples) in scientific notation truncated to 6 significant digits, hiding the exact values the sample listing exists to expose. Print integral samples as plain integers instead. Context: #27602 Assisted-by: Claude Fable 5 - - - - - 2965e0a2 by Simon Jakobi at 2026-08-15T17:09:48+02:00 DEMO: Tighten T3064 residency tolerance (do not merge) The previous demo attempts (T4830, T9675) failed for two reasons: MR pipelines pin the perf baseline to the merge-base with master (CI_MERGE_REQUEST_DIFF_BASE_SHA), and that merge-base had only one recorded sample per test env — multi-sample notes accumulate only on master commits that were the tip when a nightly pipeline ran, since nightly jobs share TEST_ENV with their regular counterparts. This branch is now rebased onto b757727a786, the newest master commit with nightly samples. Its T3064 max_bytes_used baseline on aarch64-linux-deb13-validate is bimodal (14626304 vs 13329528, mean \~13.98M), so a 1% window cannot contain either mode and the failure shows a baseline averaged from genuinely disagreeing samples. aarch64-darwin-validate and x86_64-linux-fedora43-release have two near-identical samples each, demoing the agreeing-samples output if they fail too. Context: #27602 Assisted-by: Claude Fable 5 - - - - - 20 changed files: - .gitlab/ci.sh - + changelog.d/T27586 - compiler/GHC/Tc/TyCl/PatSyn.hs - libraries/base/tests/all.T - libraries/base/tests/listThreads1.hs - libraries/base/tests/listThreads1.stdout - rts/js/thread.js - testsuite/driver/perf_notes.py - + testsuite/tests/concurrent/should_run/T16761.hs - + testsuite/tests/concurrent/should_run/T16761.stdout - testsuite/tests/concurrent/should_run/all.T - testsuite/tests/perf/compiler/all.T - testsuite/tests/rts/all.T - + testsuite/tests/vdq-rta/should_fail/T27586a.hs - + testsuite/tests/vdq-rta/should_fail/T27586a.stderr - + testsuite/tests/vdq-rta/should_fail/T27586b.hs - + testsuite/tests/vdq-rta/should_fail/T27586b.stderr - + testsuite/tests/vdq-rta/should_fail/T27586c.hs - + testsuite/tests/vdq-rta/should_fail/T27586c.stderr - testsuite/tests/vdq-rta/should_fail/all.T Changes: ===================================== .gitlab/ci.sh ===================================== @@ -1120,9 +1120,10 @@ case ${1:-help} in setup) setup && cleanup_submodules ;; configure) time_it "configure" configure ;; build_hadrian) time_it "build" build_hadrian ;; - # N.B. Always push notes, even if the build fails. This is okay to do as the - # testsuite driver doesn't record notes for tests that fail due to - # correctness. + # N.B. Always push notes, even if the build fails. Metrics from runs failing + # a perf stat check are deliberately recorded too — discarding them would + # bias the baseline towards whichever sample came first. Only correctness + # failures record nothing. test_hadrian) fetch_perf_notes res=0 ===================================== changelog.d/T27586 ===================================== @@ -0,0 +1,9 @@ +section: compiler +issues: #27586 +mrs: !16440 +synopsis: + Fix a panic on a required type argument in a pattern synonym RHS +description: + An argument of a pattern synonym that is matched against a required type + argument in the right-hand side no longer causes a panic; it is reported as + an illegal term-level use of a type variable. ===================================== compiler/GHC/Tc/TyCl/PatSyn.hs ===================================== @@ -137,7 +137,7 @@ tcInferPatSynDecl (PSB { psb_id = lname@(L _ name), psb_args = details ; (tclvl, wanted, ((lpat', args), pat_ty)) <- pushLevelAndCaptureConstraints $ tcInferPat FRRPatSynArg PatSynCtx lpat $ - mapM tcLookupId arg_names + mapM tcLookupPatSynArg arg_names ; let (ex_tvs, prov_dicts) = tcCollectEx lpat' @@ -472,7 +472,7 @@ tcCheckPatSynDecl psb@PSB{ psb_id = lname@(L _ name), psb_args = details -- location to x's binding site in lpat, namely the 'x' in Just (x,True). -- Else the error message location is wherever tcCheckPat finished, -- namely the right-hand corner of the pattern - do { arg_id <- tcLookupId arg_name + do { arg_id <- tcLookupPatSynArg arg_name ; wrap <- tcSubTypeSigma (OccurrenceOf (idName arg_id)) GenSigCtxt (idType arg_id) @@ -645,6 +645,19 @@ collectPatSynArgInfo details = InfixCon _ name1 name2 -> (map unLoc [name1, name2], True) RecCon _ names -> (map (unLoc . recordPatSynPatVar) names, False) +-- | Look up the 'Id' bound by the pattern for a declared argument of a pattern +-- synonym. With @RequiredTypeArguments@ the argument may turn out to be a type +-- variable, e.g. @pattern P x = MkT x@ where the argument of @MkT@ is a required +-- type argument; then we report an illegal term-level use of @x@ (#27586). +tcLookupPatSynArg :: Name -> TcM Id +tcLookupPatSynArg arg_name + = do { thing <- tcLookup arg_name + ; case thing of + ATcId { tct_id = id } -> return id + AGlobal (AnId id) -> return id + ATyVar {} -> failIllegalTyVar (noUserRdr arg_name) + _ -> pprPanic "tcLookupPatSynArg" (ppr arg_name) } + wrongNumberOfParmsErr :: Name -> Arity -> Arity -> TcM a wrongNumberOfParmsErr name decl_arity missing = failWithTc $ TcRnPatSynArityMismatch name decl_arity missing ===================================== libraries/base/tests/all.T ===================================== @@ -80,7 +80,7 @@ test('length001', # excessive amounts of stack space. So we specifically set a low # stack limit and mark it as failing under a few conditions. [extra_run_opts('+RTS -K8m -RTS'), - expect_fail_for(['normal', 'threaded1', 'llvm', 'nonmoving', 'nonmoving_thr', 'nonmoving_thr_ghc', 'ext-interp']), + expect_fail_for(['normal', 'threaded1', 'llvm', 'nonmoving', 'nonmoving_thr', 'nonmoving_thr_sanity', 'nonmoving_thr_ghc', 'ext-interp']), # JS doesn't support stack limit so the test sometimes passes just fine. Therefore the test is # marked as fragile. when(js_arch(), fragile(22921))], ===================================== libraries/base/tests/listThreads1.hs ===================================== @@ -2,5 +2,10 @@ module Main where import GHC.Conc.Sync +-- Regression test for the JS backend's ListThreadsOp, which used to omit the +-- running thread. Whatever other threads the RTS has is irrelevant here. main :: IO () -main = listThreads >>= print +main = do + tid <- myThreadId + ts <- listThreads + print (tid `elem` ts) ===================================== libraries/base/tests/listThreads1.stdout ===================================== @@ -1 +1 @@ -[ThreadId 1] +True ===================================== rts/js/thread.js ===================================== @@ -110,6 +110,10 @@ function h$rts_getThreadId(t) { // returns a CULLong RETURN_UBX_TUP2((t.tid / Math.pow(2,32))>>>0, (t.tid & 0xFFFFFFFF)>>>0); } +function h$eq_thread(t1,t2) { + return t1 === t2 ? 1 : 0; +} + function h$cmp_thread(t1,t2) { if(t1.tid < t2.tid) return -1; if(t1.tid > t2.tid) return 1; ===================================== testsuite/driver/perf_notes.py ===================================== @@ -84,8 +84,11 @@ PerfStat = NamedTuple('PerfStat', [('test_env', TestEnv), ('value', float)]) # A baseline recovered form stored metrics. -Baseline = NamedTuple('Baseline', [('perfStat', PerfStat), - ('commit', GitHash)]) +class Baseline(NamedTuple): + perfStat: PerfStat + commit: GitHash + # The raw samples the baseline value was averaged over. + samples: List[float] = [] # The type of exceptions which are thrown when computing the current stat value # fails. @@ -465,6 +468,10 @@ def get_allowed_changes(baseline_ref: Optional[GitRef]) -> Dict[TestName, List[A # (bool , str ) -> (str , str , str , str) -> float _commit_metric_cache = {} # type: ignore +# Like _commit_metric_cache, but mapping to the list of raw sample values the +# baseline was averaged over. Filled by get_commit_metric. +_commit_samples_cache = {} # type: ignore + # Get the baseline of a test at a given commit. This is the expected value # *before* the commit is applied (i.e. on the parent commit). # This searches git notes from older commits for recorded metrics (locally and @@ -506,7 +513,8 @@ def baseline_metric(commit: GitHash, if baseline_commit is not None: current_metric = get_commit_metric(namespace, baseline_commit, test_env, name, metric, way) if current_metric is not None: - return Baseline(current_metric, baseline_commit) + return Baseline(current_metric, baseline_commit, + get_commit_samples(namespace, baseline_commit, test_env, name, metric, way)) else: return None @@ -515,7 +523,8 @@ def baseline_metric(commit: GitHash, # Check for a metric on this commit. current_metric = get_commit_metric(namespace, current_commit, test_env, name, metric, way) if current_metric is not None: - return Baseline(current_metric, current_commit) + return Baseline(current_metric, current_commit, + get_commit_samples(namespace, current_commit, test_env, name, metric, way)) # Stop if there is an expected change at this commit. In that case # metrics on ancestor commits will not be a valid baseline. @@ -598,8 +607,28 @@ def get_commit_metric(gitNoteRef, # Save baselines to the cache. _commit_metric_cache[cacheKeyA] = baseline_by_cache_key_b + _commit_samples_cache[cacheKeyA] = values_by_cache_key_b return baseline_by_cache_key_b.get(cacheKeyB) +# Get the raw sample values that get_commit_metric averages over. Uses the +# cache filled by get_commit_metric, so no extra git calls after it has run. +def get_commit_samples(gitNoteRef, + ref: Union[GitRef, GitHash], + test_env: TestEnv, + name: TestName, + metric: MetricName, + way: WayName + ) -> List[float]: + get_commit_metric(gitNoteRef, ref, test_env, name, metric, way) + cacheKeyA = (gitNoteRef, commit_hash(ref)) + cacheKeyB = (test_env, name, metric, way) + return _commit_samples_cache.get(cacheKeyA, {}).get(cacheKeyB, []) + +# Metric samples are integral in practice; '%g' would render large byte +# counts in truncated scientific notation. +def format_samples(samples: List[float]) -> str: + return ', '.join(str(int(s)) if s == int(s) else str(s) for s in samples) + def check_stats_change(actual: PerfStat, baseline: Baseline, acceptance_window: MetricAcceptanceWindow, @@ -654,9 +683,17 @@ def check_stats_change(actual: PerfStat, ' baseline @ %s' % baseline.commit print(actual.metric, error + ':') dev = 100.0 if expected_val == 0 else round(((float(actual.value) * 100) / int(expected_val)) - 100, 1) + # A multi-sample baseline is a mean; show the samples so outliers + # corrupting the baseline are visible (#27602). + if len(baseline.samples) > 1: + samples_note = ('; baseline is mean of %d samples: %s' + % (len(baseline.samples), + format_samples(baseline.samples))) + else: + samples_note = '' change_line = (f'{actual.metric} {change.value} from {baseline.perfStat.test_env} ' f'baseline @ {baseline.commit[:7]}: {expected_val} -> {actual.value} ' - f'({dev:+g}%, allowed {acceptance_window.describe()})') + f'({dev:+g}%, allowed {acceptance_window.describe()}{samples_note})') result = failBecause('stat ' + change_line, tag='stat') if not change_allowed or force_print: @@ -666,6 +703,10 @@ def check_stats_change(actual: PerfStat, print(descr, str(val).rjust(length), extra) display(' Expected ' + full_name + ' ' + actual.metric + ':', expected_val, acceptance_window.describe()) + if len(baseline.samples) > 1: + display(' Samples ' + full_name + ' ' + actual.metric + ':', + len(baseline.samples), + '(' + format_samples(baseline.samples) + ')') display(' Lower bound ' + full_name + ' ' + actual.metric + ':', lowerBound, '') display(' Upper bound ' + full_name + ' ' + actual.metric + ':', upperBound, '') display(' Actual ' + full_name + ' ' + actual.metric + ':', actual.value, '') ===================================== testsuite/tests/concurrent/should_run/T16761.hs ===================================== @@ -0,0 +1,25 @@ +-- Test that Eq ThreadId is based on thread identity (eq_thread), +-- not on the numeric thread id, which may wrap around (#16761). +module Main (main) where + +import Control.Concurrent +import System.Mem (performGC) + +main :: IO () +main = do + t0 <- myThreadId + print (t0 == t0) + + mv <- newEmptyMVar + _ <- forkIO (myThreadId >>= putMVar mv) + tChild <- takeMVar mv + print (t0 == tChild) + print (tChild == tChild) + + -- Equality must be stable even after the GC moves the TSOs. + performGC + print (t0 == t0) + + -- Ord must agree with Eq. + print (compare t0 tChild /= EQ) + print (compare t0 t0 == EQ) ===================================== testsuite/tests/concurrent/should_run/T16761.stdout ===================================== @@ -0,0 +1,6 @@ +True +False +True +True +True +True ===================================== testsuite/tests/concurrent/should_run/all.T ===================================== @@ -310,6 +310,8 @@ test('hs_try_putmvar003', # Check forkIO exception determinism under optimization test('T13330', normal, compile_and_run, ['-O']) +test('T16761', normal, compile_and_run, ['']) + test('T26341', normal, compile_and_run, ['']) # Test EINTR for async I/O interrupted by an exception (#26341) ===================================== testsuite/tests/perf/compiler/all.T ===================================== @@ -48,7 +48,9 @@ test('T4801', ['']) test('T3064', - [collect_compiler_residency(20), + [# Demo-only: tightened from 20 so the bimodal max_bytes_used + # baseline on aarch64-linux-deb13-validate fails. + collect_compiler_residency(1), collect_compiler_runtime(2), only_ways(['normal']), ], ===================================== testsuite/tests/rts/all.T ===================================== @@ -679,7 +679,7 @@ test('T22859', [js_skip, # This test is vulnerable to changes in allocation behaviour, so we disable it in some ways when(arch('wasm32'), skip), - omit_ways(llvm_ways)], + omit_ways(llvm_ways + ['nonmoving_thr', 'nonmoving_thr_sanity'])], compile_and_run, ['-with-rtsopts -A8K']) # These tests need access to the internal RTS headers. ===================================== testsuite/tests/vdq-rta/should_fail/T27586a.hs ===================================== @@ -0,0 +1,9 @@ +{-# LANGUAGE GADTs, RequiredTypeArguments, PatternSynonyms #-} + +module T27586a where + +data T a where + MkT :: forall a -> T a + +pattern P :: Int -> T Int +pattern P x = MkT x ===================================== testsuite/tests/vdq-rta/should_fail/T27586a.stderr ===================================== @@ -0,0 +1,5 @@ +T27586a.hs:9:19: error: [GHC-01928] + • Illegal term-level use of the type variable ‘x’ + • bound at T27586a.hs:9:19 + • In the declaration for pattern synonym ‘P’ + ===================================== testsuite/tests/vdq-rta/should_fail/T27586b.hs ===================================== @@ -0,0 +1,8 @@ +{-# LANGUAGE GADTs, RequiredTypeArguments, PatternSynonyms #-} + +module T27586b where + +data T a where + MkT :: forall a -> T a + +pattern P x = MkT x ===================================== testsuite/tests/vdq-rta/should_fail/T27586b.stderr ===================================== @@ -0,0 +1,5 @@ +T27586b.hs:8:15: error: [GHC-01928] + • Illegal term-level use of the type variable ‘x’ + • bound at T27586b.hs:8:19 + • In the declaration for pattern synonym ‘P’ + ===================================== testsuite/tests/vdq-rta/should_fail/T27586c.hs ===================================== @@ -0,0 +1,9 @@ +{-# LANGUAGE GADTs, RequiredTypeArguments, PatternSynonyms #-} + +module T27586c where + +data T a where + MkT :: forall a -> T a + +pattern P :: Int -> T Int +pattern P x <- MkT x ===================================== testsuite/tests/vdq-rta/should_fail/T27586c.stderr ===================================== @@ -0,0 +1,5 @@ +T27586c.hs:9:20: error: [GHC-01928] + • Illegal term-level use of the type variable ‘x’ + • bound at T27586c.hs:9:20 + • In the declaration for pattern synonym ‘P’ + ===================================== testsuite/tests/vdq-rta/should_fail/all.T ===================================== @@ -35,3 +35,6 @@ test('T25127_fail_arity', normal, compile_fail, ['']) test('T27440e', normal, compile_fail, ['']) test('T27583f', normal, compile_fail, ['']) +test('T27586a', normal, compile_fail, ['']) +test('T27586b', normal, compile_fail, ['']) +test('T27586c', normal, compile_fail, ['']) View it on GitLab: https://gitlab.haskell.org/ghc/ghc/-/compare/a4ddab82385619a28b7fd6de1b925e1... -- View it on GitLab: https://gitlab.haskell.org/ghc/ghc/-/compare/a4ddab82385619a28b7fd6de1b925e1... You're receiving this email because of your account on gitlab.haskell.org. Manage all notifications: https://gitlab.haskell.org/-/profile/notifications | Help: https://gitlab.haskell.org/help