StgLint worth maintaining?

[Ömer Sinan Ağacan]

Created #14787 as tracking ticket. Patch is at D4404.

Ömer

2018-02-09 12:22 GMT+03:00 Simon Peyton Jones <simonpj at microsoft.com>:
> Good summary!  I suggest that you open a ticket with this email as the Description.  Then we can point to it later.
>
> I agree that there is little point in flogging a dead horse.  But there are /some/ invariants, so I vote for
> |  2. Rewrite it to only check these two and nothing else, enable it in
> |     validate (and in other build flavours that enable CoreLint).
> |
>
> Simon
>
> |  -----Original Message-----
> |  From: ghc-devs [mailto:ghc-devs-bounces at haskell.org] On Behalf Of Ömer
> |  Sinan Agacan
> |  Sent: 09 February 2018 08:42
> |  To: ghc-devs <ghc-devs at haskell.org>
> |  Subject: StgLint worth maintaining?
> |
> |  Hi,
> |
> |  I've been looking into some StgLint-related tickets:
> |
> |  - #13994: Found a StgLint problem and fixed, there's another problem
> |            waiting to be fixed. Both related with the fact that after
> |            unarisation we lose even more typing information and type
> |            checks needs to be relaxed.
> |
> |  - #14116: StgLint failed to look through newtypes, and because
> |  coercions
> |            are removed at that point it failed to type check. Solution
> |            was to relax type checks.
> |
> |  - #5345:  Because `unsafeCoerce# is operationally no-op, and we don't
> |            have coercions in STG, StgLint can't type check at all. The
> |            commit message notes:
> |
> |            > Fundamentally STG Lint is impossible, because
> |  unsafeCoerce#
> |            > can randomise all the types.
> |
> |            > This patch does a bit of fiddle faddling in StgLint which
> |            > makes it a bit better, but it's a losing battle.
> |
> |  - #14117: Related with StgLint not keeping up with recent changes
> |  (join
> |            points), because it's not enabled by default in
> |            tests/validate.
> |
> |  - #14118: Related with the fact that pre- and post-unarise we have
> |            different invariants in STG. Solution was to add a "unarise"
> |            parameter and do different checks based on that.
> |
> |  - #14120: Again type checking errors. Commit for #14116 also fixes
> |  this.
> |            The commits compares `typePrimRep`s of types instead of
> |            comparing actual types (even this is not enough, see
> |  #13994).
> |
> |  All this of course took time to debug.
> |
> |  In addition, the new `StgCSE` pass makes transformations that trigger
> |  case alternative checks (and probably some other checks) because
> |  scrutinee and result won't have same types after the transformation
> |  described in `Note [Case 2: CSEing case binders]`.
> |
> |  There's also this comment in StgLint.hs
> |
> |      WARNING:
> |      ~~~~~~~~
> |
> |      This module has suffered bit-rot; it is likely to yield lint
> |  errors
> |      for Stg code that is currently perfectly acceptable for code
> |      generation.  Solution: don't use it!  (KSW 2000-05).
> |
> |  It seems like it hasn't been used since 2000.
> |
> |  All this suggests that
> |
> |  - Checks related to types are impossible in StgLint. (see e.g. commit
> |    messages in #5345, #1420, transformations done by unariser and
> |    StgCSE)
> |
> |  - It's not enabled since 2000, which I think means that it's not
> |    needed.
> |
> |  This makes me question whether it's worth maintaining. Maybe we should
> |  just remove it.
> |
> |  If we still want to keep we should decide on what it's supposed to do.
> |  Only invariants I can think of are:
> |
> |  - After unarise there should be no unboxed tuple and sum binders.
> |
> |    unarise is a simple pass and does same thing to all binders, there
> |  are
> |    no tricky cases so I'm not sure if we need to check this.
> |
> |  - Variables should be defined before use. I again don't know if this
> |    should be checked, could this be useful for StgCSE?
> |
> |  So I think we should do one of these:
> |
> |  1. Remove StgLint.
> |
> |  2. Rewrite it to only check these two and nothing else, enable it in
> |     validate (and in other build flavours that enable CoreLint).
> |
> |  What do you think? If you think we should keep StgLint, can you think
> |  of any other checks? If we could reach a consensus I'm hoping to
> |  update StgLint (or remove it).
> |
> |  Thanks,
> |
> |  Ömer
> |  _______________________________________________
> |  ghc-devs mailing list
> |  ghc-devs at haskell.org
> |  https://na01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fmail.h
> |  askell.org%2Fcgi-bin%2Fmailman%2Flistinfo%2Fghc-
> |  devs&data=04%7C01%7Csimonpj%40microsoft.com%7C9d9affa423c84c84a25908d5
> |  6f992d87%7Cee3303d7fb734b0c8589bcd847f1c277%7C1%7C0%7C6365376260479856
> |  64%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI
> |  6Ik1haWwifQ%3D%3D%7C-
> |  1&sdata=GZ4xMoVQGyQFZxhlODBqMnWoiZrV82pqOn2ZrbvDo4U%3D&reserved=0