Add two tests for -short-paths with existential types

[shym]

As $ is used to prefix existential types, favor an alternative if it exists when -short-paths is enabled by using exactly the same penalty as for the _ prefix.

On the following program:

type _ ty = Char : char ty
type pair = Pair : 'a ty * 'a -> pair

let f = function
  | Pair (Char, x) -> x + 1

$ ocamlc -short-paths reported before:

5 |   | Pair (Char, x) -> x + 1
                          ^
Error: This expression has type $Pair_'a
       but an expression was expected of type int

while it now reports:

5 |   | Pair (Char, x) -> x + 1
                          ^
Error: This expression has type char
       but an expression was expected of type int

I'm not sure whether $ can appear in other cases than existential types.

[trefis]

-short-paths is surely going to be completely rewritten soon, but this patch is very simple, reasonable and comes with a test, so let's merge it?

[trefis]

Shouldn't this be escaped?

[shym]

tools/texquote2 does escape it automatically.

[Octachron]

@trefis I was a bit worried that the shortening logic might also happen within scope escape message leading to variations on int would escape its scope and friends.

Did you have the time to investigate this potential issue?

At the same time, many GADTs error messages are already worsened by -short-paths, so it might be not worth it to invest too much time.

[trefis]

So errr...
I used a slight variation of the given test:

type _ ty = Char : char ty
type pair = Pair : 'a ty * 'a -> pair

let f = function
  | Pair (Char, x) -> if true then x else 'd'

And got this before this PR:

$ ocamlc -version
4.14.1
$ ocamlc /tmp/sp.ml
File "/tmp/sp.ml", line 5, characters 35-36:
5 |   | Pair (Char, x) -> if true then x else 'd'
                                       ^
Error: This expression has type char but an expression was expected of type
         'a 
       This instance of char is ambiguous:
       it would escape the scope of its equation
$ ocamlc -short-paths /tmp/sp.ml
File "/tmp/sp.ml", line 5, characters 35-36:
5 |   | Pair (Char, x) -> if true then x else 'd'
                                       ^
Error: This expression has type $Pair_'a
       but an expression was expected of type 'a 
       This instance of $Pair_'a is ambiguous:
       it would escape the scope of its equation

Unless something major has changed in this area since 4.14, I'd say it can't be making things worse.

[Octachron]

Thanks for the example!
And testing with this PR, it seems that we go back to

Error: This expression has type char but an expression was expected of type
         'a 
       This instance of char is ambiguous:
       it would escape the scope of its equation

with -short-paths because the previous behavior was due to the fact that '$'<'a' in lexicographic order. In other words, the penalty hides the existential in the escape error message, which I find is a disadvantage.

[shym]

If that’s not always an improvement, maybe we should forget about it.
Would it still be useful to add the two tests to the test suite, as a way to guide the rewriting of -short-paths?

[Octachron]

Adding the two examples in the short-path test does sound like a good idea.

[shym]

I’ve renamed this PR and kept only the addition of the two tests.
I imagine a changelog entry is not really relevant for additional tests.

[Octachron]

I would add a comment that we would like short-path to print char rather than $Pair_'a in this context.

[Octachron]

While in this context, it seems better to keep the existential type.

[Octachron]

A short section title would be good to keep track of what those tests are asserting. Something like

(** Known issue: how to print existential types with equality in scope? *)

[Octachron]

I think that a change entry "testsuite: collect known issues with current -short-paths implementation" could be useful, after all good test cases are a real necessity when improving error messages.

[shym]

Thank you for all those suggestions. I’ve put you as a co-author, at that stage :-)

[gasche]

(If @Octachron is co-author then you need another approval.)