Allow top level ticked string literals
ClosedPublic

Authored by niteria on Mar 5 2018, 12:46 PM.

Details

Summary

This reverts f5b275a239d2554c4da0b7621211642bf3b10650
and changes the places that looked for Lit (MachStr _))
to use exprIsMbTickedLitString_maybe to unwrap ticks as
necessary.
Also updated relevant comments.

Test Plan

I added 3 new tests that previously reproduced.
GHC HEAD now builds with -g

Diff Detail

Repository
rGHC Glasgow Haskell Compiler
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
niteria created this revision.Mar 5 2018, 12:46 PM
bgamari added inline comments.Mar 5 2018, 2:01 PM
compiler/coreSyn/CoreUtils.hs
1737

What does "Mb" mean here?

testsuite/tests/simplCore/should_compile/T14779b.hs
81

Perhaps drop this.

niteria added inline comments.Mar 5 2018, 2:31 PM
compiler/coreSyn/CoreUtils.hs
1737

It's supposed to be "expression is maybe (optionally) ticked literal string".
I didn't like "exprIsTickedLitString" because I think it implies that there's 1 or more Ticks.
I'm very open to suggestions on naming.

testsuite/tests/simplCore/should_compile/T14779b.hs
81

Will do.

niteria updated this revision to Diff 15654.Mar 5 2018, 2:33 PM
  • rebase
  • drop Data.Data comments
  • add a (scary) test from Trac #14868
niteria edited the test plan for this revision. (Show Details)Mar 5 2018, 2:59 PM

Looks great. A couple of suggestions

compiler/coreSyn/CoreSyn.hs
406

Refer to the ticket!

compiler/coreSyn/CoreUtils.hs
1736

Suggest exprIsTickedString

1743

I suggest exprIsTickedString_maybe. And return the ByteString not the Literal. After all, it is always Lit (MachStr s) so better to return s

compiler/stgSyn/CoreToStg.hs
278

Refer to Note [CoreSyn top-level string literals] in CoreSyn.

niteria updated this revision to Diff 15655.Mar 5 2018, 3:19 PM
niteria marked 2 inline comments as done.

address @simonpj's comments

niteria marked 6 inline comments as done.Mar 5 2018, 3:49 PM
niteria updated this revision to Diff 15656.Mar 5 2018, 3:50 PM

Remove "It was because we produced a ticked top-level primitive string." as
that's misleading now.

This revision was automatically updated to reflect the committed changes.