All names are printed with their unique values by default.

[Unisay]

• [x] A test demonstrating that without a unique suffix a name doesn't survive printing/parsing.
• [x] Update printer and parser to support unique suffixes, e.g. foo#42.

Closes #4808

[effectfully]

Thank you a ton for this PR, you're doing God's work.

[bezirg]

I also disagree that it should be on by default. Don't we have some pretty debug mode for this kind of stuff? Then all the golden files could just use the debug pretty mode.

[effectfully]

There are two different things here:

1. whether the default config should prescribe to print the uniques
2. whether the golden test machinery should print them by default

I very strongly believe that when it comes to 1, the current master is straight out buggy. Pretty-printing a program and parsing it back shouldn't give you a semantically different program, that's just ridiculous. If we want to, we may pretty-print only those uniques that disambiguate names while ignoring others, but I'm not a fan of that: GHC adds "uniques" by default when it produces Core and I've never found that to be of any annoyance and I've spent quite a lot of time staring at GHC Core. So not fixing master in that regard is a non-option in my opinion, the current situation actively confuses our users (the original issue to fix the semantics changing pretty-printing exists because Runtime Verification ran into it, got very confused and, when I explained what was going on, very unhappy).

As for the second option, I feel less strongly, but I do believe that we should dump uniques into golden files by default. Your golden test is too large to have uniques? Well, then it's too large to be read anyway. Worst practices should be hard and it's not like using the non-default config is that hard. So I do believe that we should generally disincentivize creation of large golden files and if we can at the same time get correct scoping, that's a double win.

[bezirg]

I very strongly believe that when it comes to 1, the current master is straight out buggy. Pretty-printing a program and parsing it back shouldn't give you a semantically different program, that's just ridiculous.

I see now your point and I think also have ran into this before. But when does this exactly happen? Is it when there are free variables in the program or when we have to introduce fresh variables during the compilation?

[effectfully]

I see now your point and I think also have ran into this before. But when does this exactly happen? Is it when there are free variables in the program or when we have to introduce fresh variables during the compilation?

\x_0 x_1 -> x_0 being pretty-printed becomes \x x -> x, which has a different meaning. So basically any name shadowing is dangerous, which is ridiculous.

[Unisay]

I agree with:

• Default pretty printing config should produce a representation that always parses back (roundtrip);
• There might be some cases where we explicitly opt-out of printing indexes, it should be possible;
• The usage of golden files should be minimised and having to make an extra step to opt-out of indexes could have a positive effect in this dimension (aka "Worst practices should be hard")
• The "fragility" concern is real: if only indexes have changed but program didn't, then we don't want to see such change in the diff. @effectfully suggested that rename pass addresses this.

[zliu41]

I agree should fix 1. For 2, I really disagree - uniques in anything but the tiniest golden files are quite annoying. Added a let binding? Everything afterwards will have different uniques. Added a rename in one of the passes? Congratulations, every single unique in every single golden file will now be different. What benefit do we get for such trouble?

[Unisay]

I am in the process of re-working this PR in order to have a cake and eat it too:

• Unique indexes should be printed by default;
• Golden tests explicitly opt-in to a simple representation that doesn't print unique indexes;

This entails quite some boring work: the change isn't hard but its a lot files to visit and decide. Stay tuned.

[effectfully]

now have additional redundant parentheses which appears to be what makes the line count so large.

I promised to take a look at it and got caught up with other stuff. Gonna do it now.

[effectfully]

@Unisay you've changed the behavior of prettyPirReadable in PlutusIR.Core.Instance.Pretty.Readable by turning botPrettyConfigReadable into topPrettyConfigReadable. So making it

-- | Pretty-print something with the @PrettyConfigReadable@ config.
prettyPirReadable :: PrettyPir a => a -> Doc ann
prettyPirReadable = prettyBy (botPrettyConfigReadable prettyConfigName def)

-- | Pretty-print something with the @PrettyConfigReadableSimple@ config.
prettyPirReadableSimple :: PrettyPir a => a -> Doc ann
prettyPirReadableSimple = prettyBy (botPrettyConfigReadable prettyConfigNameSimple def)

will solve the extra parens issue. BTW, we don't have two sets of parens there, one of them are there because a let-expression is applied to a let-expression, i.e. (let x = a in q) (let y = b in r), i.e. those parens are as expected. Only took me a few thousand pulled out hairs to figure out, phew.