Simon Jakobi pushed to branch wip/sjakobi/T27602 at Glasgow Haskell Compiler / GHC

Commits:

20 changed files:

Changes:

  • .gitlab/ci.sh
    ... ... @@ -1120,9 +1120,10 @@ case ${1:-help} in
    1120 1120 
       setup) setup && cleanup_submodules ;;
    1121 1121 
       configure) time_it "configure" configure ;;
    1122 1122 
       build_hadrian) time_it "build" build_hadrian ;;
    1123 
    -  # N.B. Always push notes, even if the build fails. This is okay to do as the
    1124 
    -  # testsuite driver doesn't record notes for tests that fail due to
    1125 
    -  # correctness.
    1123 
    +  # N.B. Always push notes, even if the build fails. Metrics from runs failing
    1124 
    +  # a perf stat check are deliberately recorded too — discarding them would
    1125 
    +  # bias the baseline towards whichever sample came first. Only correctness
    1126 
    +  # failures record nothing.
    1126 1127 
       test_hadrian)
    1127 1128 
         fetch_perf_notes
    1128 1129 
         res=0

  • changelog.d/T27586
    1 
    +section: compiler
    2 
    +issues: #27586
    3 
    +mrs: !16440
    4 
    +synopsis:
    5 
    +  Fix a panic on a required type argument in a pattern synonym RHS
    6 
    +description:
    7 
    +  An argument of a pattern synonym that is matched against a required type
    8 
    +  argument in the right-hand side no longer causes a panic; it is reported as
    9 
    +  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
    137 137 
            ; (tclvl, wanted, ((lpat', args), pat_ty))
    138 138 
                 <- pushLevelAndCaptureConstraints      $
    139 139 
                    tcInferPat FRRPatSynArg PatSynCtx lpat $
    140 
    -               mapM tcLookupId arg_names
    140 
    +               mapM tcLookupPatSynArg arg_names
    141 141
    142 142 
            ; let (ex_tvs, prov_dicts) = tcCollectEx lpat'
    143 143
    ... ... @@ -472,7 +472,7 @@ tcCheckPatSynDecl psb@PSB{ psb_id = lname@(L _ name), psb_args = details
    472 472 
                -- location to x's binding site in lpat, namely the 'x' in Just (x,True).
    473 473 
                -- Else the error message location is wherever tcCheckPat finished,
    474 474 
                -- namely the right-hand corner of the pattern
    475 
    -        do { arg_id <- tcLookupId arg_name
    475 
    +        do { arg_id <- tcLookupPatSynArg arg_name
    476 476 
                ; wrap <- tcSubTypeSigma (OccurrenceOf (idName arg_id))
    477 477 
                                         GenSigCtxt
    478 478 
                                         (idType arg_id)
    ... ... @@ -645,6 +645,19 @@ collectPatSynArgInfo details =
    645 645 
         InfixCon _ name1 name2 -> (map unLoc [name1, name2], True)
    646 646 
         RecCon _ names         -> (map (unLoc . recordPatSynPatVar) names, False)
    647 647
    648 
    +-- | Look up the 'Id' bound by the pattern for a declared argument of a pattern
    649 
    +-- synonym. With @RequiredTypeArguments@ the argument may turn out to be a type
    650 
    +-- variable, e.g. @pattern P x = MkT x@ where the argument of @MkT@ is a required
    651 
    +-- type argument; then we report an illegal term-level use of @x@ (#27586).
    652 
    +tcLookupPatSynArg :: Name -> TcM Id
    653 
    +tcLookupPatSynArg arg_name
    654 
    +  = do { thing <- tcLookup arg_name
    655 
    +       ; case thing of
    656 
    +           ATcId { tct_id = id } -> return id
    657 
    +           AGlobal (AnId id)     -> return id
    658 
    +           ATyVar {}             -> failIllegalTyVar (noUserRdr arg_name)
    659 
    +           _                     -> pprPanic "tcLookupPatSynArg" (ppr arg_name) }
    660 
    +
    648 661 
     wrongNumberOfParmsErr :: Name -> Arity -> Arity -> TcM a
    649 662 
     wrongNumberOfParmsErr name decl_arity missing
    650 663 
       = failWithTc $ TcRnPatSynArityMismatch name decl_arity missing

  • libraries/base/tests/all.T
    ... ... @@ -80,7 +80,7 @@ test('length001',
    80 80 
          # excessive amounts of stack space. So we specifically set a low
    81 81 
          # stack limit and mark it as failing under a few conditions.
    82 82 
          [extra_run_opts('+RTS -K8m -RTS'),
    83 
    -     expect_fail_for(['normal', 'threaded1', 'llvm', 'nonmoving', 'nonmoving_thr', 'nonmoving_thr_ghc', 'ext-interp']),
    83 
    +     expect_fail_for(['normal', 'threaded1', 'llvm', 'nonmoving', 'nonmoving_thr', 'nonmoving_thr_sanity', 'nonmoving_thr_ghc', 'ext-interp']),
    84 84 
          # JS doesn't support stack limit so the test sometimes passes just fine. Therefore the test is
    85 85 
          # marked as fragile.
    86 86 
          when(js_arch(), fragile(22921))],

  • libraries/base/tests/listThreads1.hs
    ... ... @@ -2,5 +2,10 @@ module Main where
    2 2
    3 3 
     import GHC.Conc.Sync
    4 4
    5 
    +-- Regression test for the JS backend's ListThreadsOp, which used to omit the
    6 
    +-- running thread. Whatever other threads the RTS has is irrelevant here.
    5 7 
     main :: IO ()
    6 
    -main = listThreads >>= print
    8 
    +main = do
    9 
    +  tid <- myThreadId
    10 
    +  ts <- listThreads
    11 
    +  print (tid `elem` ts)

  • libraries/base/tests/listThreads1.stdout
    1 
    -[ThreadId 1]
    1 
    +True

  • rts/js/thread.js
    ... ... @@ -110,6 +110,10 @@ function h$rts_getThreadId(t) { // returns a CULLong
    110 110 
       RETURN_UBX_TUP2((t.tid / Math.pow(2,32))>>>0, (t.tid & 0xFFFFFFFF)>>>0);
    111 111 
     }
    112 112
    113 
    +function h$eq_thread(t1,t2) {
    114 
    +  return t1 === t2 ? 1 : 0;
    115 
    +}
    116 
    +
    113 117 
     function h$cmp_thread(t1,t2) {
    114 118 
       if(t1.tid < t2.tid) return -1;
    115 119 
       if(t1.tid > t2.tid) return 1;

  • testsuite/driver/perf_notes.py
    ... ... @@ -84,8 +84,11 @@ PerfStat = NamedTuple('PerfStat', [('test_env', TestEnv),
    84 84 
                                        ('value', float)])
    85 85
    86 86 
     # A baseline recovered form stored metrics.
    87 
    -Baseline = NamedTuple('Baseline', [('perfStat', PerfStat),
    88 
    -                                   ('commit', GitHash)])
    87 
    +class Baseline(NamedTuple):
    88 
    +    perfStat: PerfStat
    89 
    +    commit: GitHash
    90 
    +    # The raw samples the baseline value was averaged over.
    91 
    +    samples: List[float] = []
    89 92
    90 93 
     # The type of exceptions which are thrown when computing the current stat value
    91 94 
     # fails.
    ... ... @@ -465,6 +468,10 @@ def get_allowed_changes(baseline_ref: Optional[GitRef]) -> Dict[TestName, List[A
    465 468 
     # (bool          , str   ) -> (str     , str , str   , str) -> float
    466 469 
     _commit_metric_cache = {} # type: ignore
    467 470
    471 
    +# Like _commit_metric_cache, but mapping to the list of raw sample values the
    472 
    +# baseline was averaged over. Filled by get_commit_metric.
    473 
    +_commit_samples_cache = {} # type: ignore
    474 
    +
    468 475 
     # Get the baseline of a test at a given commit. This is the expected value
    469 476 
     # *before* the commit is applied (i.e. on the parent commit).
    470 477 
     # This searches git notes from older commits for recorded metrics (locally and
    ... ... @@ -506,7 +513,8 @@ def baseline_metric(commit: GitHash,
    506 513 
             if baseline_commit is not None:
    507 514 
                 current_metric = get_commit_metric(namespace, baseline_commit, test_env, name, metric, way)
    508 515 
                 if current_metric is not None:
    509 
    -                return Baseline(current_metric, baseline_commit)
    516 
    +                return Baseline(current_metric, baseline_commit,
    517 
    +                                get_commit_samples(namespace, baseline_commit, test_env, name, metric, way))
    510 518 
                 else:
    511 519 
                     return None
    512 520
    ... ... @@ -515,7 +523,8 @@ def baseline_metric(commit: GitHash,
    515 523 
                 # Check for a metric on this commit.
    516 524 
                 current_metric = get_commit_metric(namespace, current_commit, test_env, name, metric, way)
    517 525 
                 if current_metric is not None:
    518 
    -                return Baseline(current_metric, current_commit)
    526 
    +                return Baseline(current_metric, current_commit,
    527 
    +                                get_commit_samples(namespace, current_commit, test_env, name, metric, way))
    519 528
    520 529 
                 # Stop if there is an expected change at this commit. In that case
    521 530 
                 # metrics on ancestor commits will not be a valid baseline.
    ... ... @@ -598,8 +607,28 @@ def get_commit_metric(gitNoteRef,
    598 607
    599 608 
         # Save baselines to the cache.
    600 609 
         _commit_metric_cache[cacheKeyA] = baseline_by_cache_key_b
    610 
    +    _commit_samples_cache[cacheKeyA] = values_by_cache_key_b
    601 611 
         return baseline_by_cache_key_b.get(cacheKeyB)
    602 612
    613 
    +# Get the raw sample values that get_commit_metric averages over. Uses the
    614 
    +# cache filled by get_commit_metric, so no extra git calls after it has run.
    615 
    +def get_commit_samples(gitNoteRef,
    616 
    +                       ref: Union[GitRef, GitHash],
    617 
    +                       test_env: TestEnv,
    618 
    +                       name: TestName,
    619 
    +                       metric: MetricName,
    620 
    +                       way: WayName
    621 
    +                       ) -> List[float]:
    622 
    +    get_commit_metric(gitNoteRef, ref, test_env, name, metric, way)
    623 
    +    cacheKeyA = (gitNoteRef, commit_hash(ref))
    624 
    +    cacheKeyB = (test_env, name, metric, way)
    625 
    +    return _commit_samples_cache.get(cacheKeyA, {}).get(cacheKeyB, [])
    626 
    +
    627 
    +# Metric samples are integral in practice; '%g' would render large byte
    628 
    +# counts in truncated scientific notation.
    629 
    +def format_samples(samples: List[float]) -> str:
    630 
    +    return ', '.join(str(int(s)) if s == int(s) else str(s) for s in samples)
    631 
    +
    603 632 
     def check_stats_change(actual: PerfStat,
    604 633 
                            baseline: Baseline,
    605 634 
                            acceptance_window: MetricAcceptanceWindow,
    ... ... @@ -654,9 +683,17 @@ def check_stats_change(actual: PerfStat,
    654 683 
                     ' baseline @ %s' % baseline.commit
    655 684 
             print(actual.metric, error + ':')
    656 685 
             dev = 100.0 if expected_val == 0 else round(((float(actual.value) * 100) / int(expected_val)) - 100, 1)
    686 
    +        # A multi-sample baseline is a mean; show the samples so outliers
    687 
    +        # corrupting the baseline are visible (#27602).
    688 
    +        if len(baseline.samples) > 1:
    689 
    +            samples_note = ('; baseline is mean of %d samples: %s'
    690 
    +                            % (len(baseline.samples),
    691 
    +                               format_samples(baseline.samples)))
    692 
    +        else:
    693 
    +            samples_note = ''
    657 694 
             change_line = (f'{actual.metric} {change.value} from {baseline.perfStat.test_env} '
    658 695 
                            f'baseline @ {baseline.commit[:7]}: {expected_val} -> {actual.value} '
    659 
    -                       f'({dev:+g}%, allowed {acceptance_window.describe()})')
    696 
    +                       f'({dev:+g}%, allowed {acceptance_window.describe()}{samples_note})')
    660 697 
             result = failBecause('stat ' + change_line, tag='stat')
    661 698
    662 699 
         if not change_allowed or force_print:
    ... ... @@ -666,6 +703,10 @@ def check_stats_change(actual: PerfStat,
    666 703 
                 print(descr, str(val).rjust(length), extra)
    667 704
    668 705 
             display('    Expected    ' + full_name + ' ' + actual.metric + ':', expected_val, acceptance_window.describe())
    706 
    +        if len(baseline.samples) > 1:
    707 
    +            display('    Samples     ' + full_name + ' ' + actual.metric + ':',
    708 
    +                    len(baseline.samples),
    709 
    +                    '(' + format_samples(baseline.samples) + ')')
    669 710 
             display('    Lower bound ' + full_name + ' ' + actual.metric + ':', lowerBound, '')
    670 711 
             display('    Upper bound ' + full_name + ' ' + actual.metric + ':', upperBound, '')
    671 712 
             display('    Actual      ' + full_name + ' ' + actual.metric + ':', actual.value, '')

  • testsuite/tests/concurrent/should_run/T16761.hs
    1 
    +-- Test that Eq ThreadId is based on thread identity (eq_thread),
    2 
    +-- not on the numeric thread id, which may wrap around (#16761).
    3 
    +module Main (main) where
    4 
    +
    5 
    +import Control.Concurrent
    6 
    +import System.Mem (performGC)
    7 
    +
    8 
    +main :: IO ()
    9 
    +main = do
    10 
    +  t0 <- myThreadId
    11 
    +  print (t0 == t0)
    12 
    +
    13 
    +  mv <- newEmptyMVar
    14 
    +  _ <- forkIO (myThreadId >>= putMVar mv)
    15 
    +  tChild <- takeMVar mv
    16 
    +  print (t0 == tChild)
    17 
    +  print (tChild == tChild)
    18 
    +
    19 
    +  -- Equality must be stable even after the GC moves the TSOs.
    20 
    +  performGC
    21 
    +  print (t0 == t0)
    22 
    +
    23 
    +  -- Ord must agree with Eq.
    24 
    +  print (compare t0 tChild /= EQ)
    25 
    +  print (compare t0 t0 == EQ)

  • testsuite/tests/concurrent/should_run/T16761.stdout
    1 
    +True
    2 
    +False
    3 
    +True
    4 
    +True
    5 
    +True
    6 
    +True

  • testsuite/tests/concurrent/should_run/all.T
    ... ... @@ -310,6 +310,8 @@ test('hs_try_putmvar003',
    310 310 
     # Check forkIO exception determinism under optimization
    311 311 
     test('T13330', normal, compile_and_run, ['-O'])
    312 312
    313 
    +test('T16761', normal, compile_and_run, [''])
    314 
    +
    313 315 
     test('T26341', normal, compile_and_run, [''])
    314 316
    315 317 
     # Test EINTR for async I/O interrupted by an exception (#26341)

  • testsuite/tests/perf/compiler/all.T
    ... ... @@ -48,7 +48,9 @@ test('T4801',
    48 48 
          [''])
    49 49
    50 50 
     test('T3064',
    51 
    -     [collect_compiler_residency(20),
    51 
    +     [# Demo-only: tightened from 20 so the bimodal max_bytes_used
    52 
    +      # baseline on aarch64-linux-deb13-validate fails.
    53 
    +      collect_compiler_residency(1),
    52 54 
           collect_compiler_runtime(2),
    53 55 
           only_ways(['normal']),
    54 56 
           ],

  • testsuite/tests/rts/all.T
    ... ... @@ -679,7 +679,7 @@ test('T22859',
    679 679 
          [js_skip,
    680 680 
           # This test is vulnerable to changes in allocation behaviour, so we disable it in some ways
    681 681 
           when(arch('wasm32'), skip),
    682 
    -      omit_ways(llvm_ways)],
    682 
    +      omit_ways(llvm_ways + ['nonmoving_thr', 'nonmoving_thr_sanity'])],
    683 683 
          compile_and_run, ['-with-rtsopts -A8K'])
    684 684
    685 685 
     # These tests need access to the internal RTS headers.

  • testsuite/tests/vdq-rta/should_fail/T27586a.hs
    1 
    +{-# LANGUAGE GADTs, RequiredTypeArguments, PatternSynonyms #-}
    2 
    +
    3 
    +module T27586a where
    4 
    +
    5 
    +data T a where
    6 
    +  MkT :: forall a -> T a
    7 
    +
    8 
    +pattern P :: Int -> T Int
    9 
    +pattern P x = MkT x

  • testsuite/tests/vdq-rta/should_fail/T27586a.stderr
    1 
    +T27586a.hs:9:19: error: [GHC-01928]
    2 
    +    • Illegal term-level use of the type variable ‘x’
    3 
    +    • bound at T27586a.hs:9:19
    4 
    +    • In the declaration for pattern synonym ‘P’
    5 
    +

  • testsuite/tests/vdq-rta/should_fail/T27586b.hs
    1 
    +{-# LANGUAGE GADTs, RequiredTypeArguments, PatternSynonyms #-}
    2 
    +
    3 
    +module T27586b where
    4 
    +
    5 
    +data T a where
    6 
    +  MkT :: forall a -> T a
    7 
    +
    8 
    +pattern P x = MkT x

  • testsuite/tests/vdq-rta/should_fail/T27586b.stderr
    1 
    +T27586b.hs:8:15: error: [GHC-01928]
    2 
    +    • Illegal term-level use of the type variable ‘x’
    3 
    +    • bound at T27586b.hs:8:19
    4 
    +    • In the declaration for pattern synonym ‘P’
    5 
    +

  • testsuite/tests/vdq-rta/should_fail/T27586c.hs
    1 
    +{-# LANGUAGE GADTs, RequiredTypeArguments, PatternSynonyms #-}
    2 
    +
    3 
    +module T27586c where
    4 
    +
    5 
    +data T a where
    6 
    +  MkT :: forall a -> T a
    7 
    +
    8 
    +pattern P :: Int -> T Int
    9 
    +pattern P x <- MkT x

  • testsuite/tests/vdq-rta/should_fail/T27586c.stderr
    1 
    +T27586c.hs:9:20: error: [GHC-01928]
    2 
    +    • Illegal term-level use of the type variable ‘x’
    3 
    +    • bound at T27586c.hs:9:20
    4 
    +    • In the declaration for pattern synonym ‘P’
    5 
    +

  • testsuite/tests/vdq-rta/should_fail/all.T
    ... ... @@ -35,3 +35,6 @@ test('T25127_fail_arity', normal, compile_fail, [''])
    35 35
    36 36 
     test('T27440e', normal, compile_fail, [''])
    37 37 
     test('T27583f', normal, compile_fail, [''])
    38 
    +test('T27586a', normal, compile_fail, [''])
    39 
    +test('T27586b', normal, compile_fail, [''])
    40 
    +test('T27586c', normal, compile_fail, [''])

    • —
    View it on GitLab.
    You're receiving this email because of your account on gitlab.haskell.org. Manage all notifications · Help Notification message regarding https://gitlab.haskell.org/ghc/ghc/-/compare/a4ddab82385619a28b7fd6de1b925e13c77964d5...2965e0a244131ceafd4982bd02858104946cf7a0 at 1786808088