Give more precise error when disambiguation could not possibly work

[lpw25]

Constructor disambiguation can lead to confusing error messages, like:

module M = struct
  type t = A | B | C
  let get _ _ = A 
end

let () =
  match M.get () with
  | A | B | C -> ();;

Line 8, characters 4-5:
8 |   | A | B | C -> ();;
        ^
Error: Unbound constructor A

Here, the mistake was clearly missing an argument in the call to M.get, but the error message is complaining about an unbound constructor.

This PR adds a more specific error message for cases where disambiguation could not possibly work. For example, the above now gives:

Line 8, characters 4-5:
8 |   | A | B | C -> ();;
        ^
Error: This pattern should not be a constructor, the expected type is
       'a -> M.t

[gasche]

I haven't done a complete review yet but this looks very promising. The code is easier to follow, the naming is nice, getting rid of exceptions is nice and the error messages are better.

@lpw25, any chance that you would be willing to have a look at #10307? It's in the same broad design space (but less user-facing), I even wondered if the two would conflict (but I don't think so), and it's a fairly small change.

[gasche]

for me an un-labelled boolean argument is a code smell. You could use get_path loc ~expectation:true ty_expected.

But maybe there is an even clearer way to distinguish between the two situations here? On a first read I'm not sure I understand what Expr_not_a_record_type means compared to Wrong_expected_kind, and why we don't have a symmetric case for (non-polymorphic) variant constructors.

[gasche]

Could this suggest that type_forcing_context should be extended with a Record_project case or something?

[lpw25]

This code was getting hard to follow so I refactored it. Should be clearer now.

The difference between the two errors is that one is about using a constructor with an inappropriate expected type, and the other is about using a projection on an expression of the wrong type. You only get the second kind of error for records since there are no projections for variants.

[Octachron]

Note that this also seems to be a better alternative to #9395 .

Overall, with type-directed disambiguation in mind, it seems to be a good idea to raise an error early due to a kind mismatch.

[lpw25]

Rebased and addressed review comments.

[Octachron]

Raising an error early seems to be the best choice in an environment where type-directed disambiguation is prevalent. In particular, the new error message should avoid few wild-goose chase for unbound constructors (or fields).

The code itself is easier to follow.

I still think that the literal handling is a bit adhoc, but this is not the only place where this happens, and I agree that this makes the error message much more understandable for non-expert users.

[lpw25]

Changes entry added -- should be good to merge.