refactor: remove unnecessary instance and use of unsafeCoerce

[MangoIV]

• unsafeCoerce was used where eqTypeRep could be used, this was corrected
• an incorrect instance for NFData existed which was removed because it's unused

[MangoIV]

Access violation in generated code when reading 0x241a9c45d59

 Attempting to reconstruct a stack trace...

hls-hlint-plugin-tests.exe: <file descriptor: 6>: hPutBuf: resource vanished (Broken pipe)
   Frame	Code address
 * 0x8430efd710	0x7ff7e56985a7 D:\a\haskell-language-server\haskell-language-server\dist-newstyle\build\x86_64-windows\ghc-9.8.4\hls-2.9.0.1\t\hls-hlint-plugin-tests\build\hls-hlint-plugin-tests\hls-hlint-plugin-tests.exe+0x31985a7
 * 0x8430efd750	0x7ff7e568cf17 D:\a\haskell-language-server\haskell-language-server\dist-newstyle\build\x86_64-windows\ghc-9.8.4\hls-2.9.0.1\t\hls-hlint-plugin-tests\build\hls-hlint-plugin-tests\hls-hlint-plugin-tests.exe+0x318cf17
 * 0x8430efd758	0x7ff7e57da179 D:\a\haskell-language-server\haskell-language-server\dist-newstyle\build\x86_64-windows\ghc-9.8.4\hls-2.9.0.1\t\hls-hlint-plugin-tests\build\hls-hlint-plugin-tests\hls-hlint-plugin-tests.exe+0x32da179
 * 0x8430efd760	0x24100000003
 * 0x8430efd768	0x241a9c1eb70
 * 0x8430efd770	0x2000
 * 0x8430efd778	0x7ef48f3575c8

this doesn't looks like it's a problem caused by me, is it?

[fendor]

Thanks for your contribution!

this doesn't looks like it's a problem caused by me, is it?

It is not caused by you, you can ignore it! :)

[fendor]

LGTM.

I am wondering whether this has an impact on performance, @MangoIV did you happen to look into core and see if anything changed?

[MangoIV]

If there’s no overlappable instances in play I don’t think it’s possible for this to have a performance impact. I think you were right, this instance was indeed unused.

[MangoIV]

And the overlappable instances that would work over the one removed would really not be recommendable. Like NFData (f a) forall f a

[fendor]

I meant your correct type coercion which was previously using unsafeCoerce.

I have no concerns that the NFData is unused.

[MangoIV]

I meant your correct type coercion which was previously using unsafeCoerce.

I can check but if anything this should do less work, right?

[fendor]

A performance win would be pretty nice, though :)

[MangoIV]

On master

$wfromKeyType
  = \ ww_s2pBc ->
      case $wlookupKeyValue ww_s2pBc of
      { KeyValue @a_a2o3r $dTypeable_a2o3s $dHashable_a2o3t $dShow_a2o3u
                 a1_a2o3v ds_a2o3w ->
      case splitApp ($dTypeable_a2o3s `cast` <Co:3> :: ...) of {
        IsApp @k'_i2o3T @f1_i2o3U @x_i2o3V co_i2o3W f2_i2o3X x1_i2o3Y ->
          case splitApp f2_i2o3X of {
            IsApp @ipv_i2o7p @ipv1_i2o7q @ipv2_i2o7r ipv3_i2o7s ipv4_i2o7t
                  ipv5_i2o7u ->
              Nothing;
            IsCon irred_i2o7w con_i2o7x ds4_i2o7y ->
              case con_i2o7x of
              { TyCon hi1_i2o7D lo1_i2o7E ds2_i2o7F ds3_i2o7G ds1_i2o7H
                      ds5_i2o7I ->
              case fromKeyType1 of
              { TyCon hi2_i2o7L lo2_i2o7M ds6_i2o7N ds7_i2o7O ds8_i2o7P
                      ds9_i2o7Q ->
              case eqWord64# hi1_i2o7D hi2_i2o7L of {
                __DEFAULT -> Nothing;
                1# ->
                  case eqWord64# lo1_i2o7E lo2_i2o7M of {
                    __DEFAULT -> Nothing;
                    1# ->
                      case unsafeEqualityProof of { UnsafeRefl co1_a2oP0 ->
                      case a1_a2o3v `cast` <Co:6> :: ... of { (ds12_d2nVe, f_a2loB) ->
                      Just ($WSomeTypeRep x1_i2o3Y, f_a2loB)
                      }
                      }
                  }
              }
              }
              }
          };
        IsCon ipv_i2o44 ipv1_i2o45 ipv2_i2o46 -> Nothing
      }
      }

fromKeyType
  = \ ds_s2pBa ->
      case ds_s2pBa `cast` <Co:1> :: ... of { I# ww_s2pBc ->
      $wfromKeyType ww_s2pBc
      }

on this branch

$wfromKeyType
  = \ ww_sYEm ->
      case $wlookupKeyValue ww_sYEm of
      { KeyValue @a_aYkH $dTypeable_aYkI $dHashable_aYkJ $dShow_aYkK
                 a1_aYkL ds_aYkM ->
      case splitApp ($dTypeable_aYkI `cast` <Co:3> :: ...) of {
        IsApp @k'_iYl9 @f1_iYla @x_iYlb co_iYlc f2_iYld x1_iYle ->
          case sameTypeRep f2_iYld fromKey1 of {
            False -> Nothing;
            True ->
              case unsafeEqualityProof of { UnsafeRefl co1_iYwt ->
              case a1_aYkL `cast` <Co:42> :: ... of { (ds1_dXU3, f_aCSF) ->
              Just ($WSomeTypeRep x1_iYle, f_aCSF)
              }
              }
          };
        IsCon ipv_iYlk ipv1_iYll ipv2_iYlm -> Nothing
      }
      }

fromKeyType
  = \ ds_sYEk ->
      case ds_sYEk `cast` <Co:1> :: ... of { I# ww_sYEm ->
      $wfromKeyType ww_sYEm
      }

[MangoIV]

the former inlined more, now sameTypeRep is not inlined anymore. I wonder if that makes things worse.

[MangoIV]

the documentation says:

We need INLINEABLE on the eqTypeRep and decTypeRep as GHC
seems to want to inline sameTypeRep here, making tham bigger.
By exposing exact RHS, they stay small and other optimizations may
fire first, so GHC can realise that inlining sameTypeRep is often
(but not always) a bad idea.

So GHC behaves as expected here. I wonder why == inlined then.

[MangoIV]

I guess it could actually be slightly worse :/

[fendor]

Benchmarking is probably the only way to know for sure

[MangoIV]

I guess. I still think there's a high probability that it's now faster because sameTypeRep itself uses the fingerprints directly and does not case. But yeah, we allocate an additional Bool and also we have to call sameTypeRep Idk. Do you want me to write a benchmark?

[fendor]

No, I think it is fine. I was just curious whether we can estimate the impact.