Update locations during destructive substitutions

[trefis]

This PR proposes to update locations when performing a destructive substitution, this is to avoid error messages like this one:

$ cat test.ml
module M : Set.S with type elt := unit = Set.Make (String)
$ ocamlc -c ./test.ml
File "./test.ml", line 1, characters 41-58:
Error: Signature mismatch:
       ...
       Values do not match:
         val mem : elt -> t -> bool
       is not included in
         val mem : unit -> t -> bool
       File "set.mli", line 77, characters 4-29: Expected declaration
       File "set.mli", line 77, characters 4-29: Actual declaration

After this PR, it becomes:

File "./test.ml", line 1, characters 41-58:
Error: Signature mismatch:
       ...
       Values do not match:
         val mem : elt -> t -> bool
       is not included in
         val mem : unit -> t -> bool
       File "./test.ml", line 1, characters 11-38: Expected declaration
       File "set.mli", line 77, characters 4-29: Actual declaration

[trefis]

PS: a bootstrap was apparently necessary (which surprised me a little) otherwise this would happen:

../../boot/ocamlrun ../../ocamlc -nostdlib -I ../../stdlib -I ../../utils -I ../../typing -I ../../bytecomp -I ../../asmcomp -absname -w +a-4-9-41-42-44-45-48 -bin-annot -g -I ../../stdlib -warn-error A -safe-string -strict-sequence -strict-formats -pack -o dynlinkaux.cmo ../../utils/misc.cmo ../../utils/config.cmo ../../utils/identifiable.cmo ../../utils/numbers.cmo ../../utils/arg_helper.cmo ../../utils/clflags.cmo ../../utils/tbl.cmo ../../utils/consistbl.cmo ../../utils/terminfo.cmo ../../utils/warnings.cmo ../../utils/build_path_prefix_map.cmo ../../parsing/asttypes.cmi ../../parsing/location.cmo ../../parsing/longident.cmo ../../parsing/docstrings.cmo ../../parsing/syntaxerr.cmo ../../parsing/ast_helper.cmo ../../parsing/ast_mapper.cmo ../../parsing/ast_iterator.cmo ../../parsing/attr_helper.cmo ../../parsing/builtin_attributes.cmo ../../typing/ident.cmo ../../typing/path.cmo ../../typing/primitive.cmo ../../typing/types.cmo ../../typing/btype.cmo ../../typing/subst.cmo ../../typing/predef.cmo ../../typing/datarepr.cmo ../../typing/cmi_format.cmo ../../typing/env.cmo ../../bytecomp/lambda.cmo ../../bytecomp/instruct.cmo ../../bytecomp/cmo_format.cmi ../../bytecomp/opcodes.cmo ../../bytecomp/runtimedef.cmo ../../bytecomp/bytesections.cmo ../../bytecomp/dll.cmo ../../bytecomp/meta.cmo ../../bytecomp/symtable.cmo
Fatal error: exception Invalid_argument("output_value: abstract value (outside heap)")
Raised by primitive operation at unknown location
Called from file "bytecomp/bytepackager.ml", line 249, characters 6-40
Re-raised at file "bytecomp/bytepackager.ml", line 277, characters 10-11
Called from file "bytecomp/bytepackager.ml", line 294, characters 6-67
Re-raised at file "bytecomp/bytepackager.ml", line 296, characters 36-37
Called from file "driver/main.ml", line 176, characters 6-94
Re-raised at file "parsing/location.ml", line 494, characters 22-25
Re-raised at file "parsing/location.ml", line 494, characters 22-25
Re-raised at file "parsing/location.ml", line 494, characters 22-25
Re-raised at file "parsing/location.ml", line 494, characters 22-25
Re-raised at file "parsing/location.ml", line 494, characters 22-25
Re-raised at file "parsing/location.ml", line 494, characters 22-25
Called from file "driver/main.ml", line 202, characters 4-35
Called from file "driver/main.ml", line 206, characters 2-9
make[3]: *** [dynlinkaux.cmo] Error 2

[trefis]

ping @Drup , @lpw25 : does one of you have time to review this?

[Drup]

I reviewed. The code looks fine and is pretty low risk (it only change location on things where locations used to be, well, mostly crap). Also, it works for both non-destructive and destructive substitutions.

The tests, however, could be improved:

• It's missing the part of the information that it is supposed to test. In particular the error message you gave in the description is longer and more precise than the one in the testsuite.
• It'll break when someone extends Map.Make. It should probably use a made-up local module.

[trefis]

It's missing the part of the information that it is supposed to test. In particular the error message you gave in the description is longer and more precise than the one in the testsuite.

Yes, that's because when printing error messages some locations aren't printed if they come from the toplevel. I rewrote the test to switch from using expect_test to ocamlc, the error message is now similar to the one given in the description of this PR.

It'll break when someone extends Map.Make. It should probably use a made-up local module.

Fair point, done.

[Drup]

The ocamltest script doesn't look very nice, but the tests are great! Looks good to me.