@alainfrisch Please could you comment?

Some observations:

• In the compiler, input_name is used only to report "unit-wide" error messages (cf !input_name in Location).

• It could be used in other tools such as ocamldoc (which you treated) or ppx rewriters (e.g. to insert the name of the source file somewhere in the Parsetree).

• The suggested change impacts Pparse.{parse_implementation,parse_interface} (leaving Location.input_name equal to the original file name rather than some intermediate post-processed source file), and also Pparse.file (used by ocamldoc and ocamldep, but also perhaps by external tools), which leaves input_name untouched (i.e. possibly undefined).

It's hard to evaluate the risk of introducing problems in external tools with the change. The risk might be reduced a bit if Location.input_name is set in Location.preprocess (which is usually called explicitly before Pparse.file) rather than in Location.parse_file. Typically, this might avoid possible problems with error reporting in ocamldep with the current proposal.

Now, is the change a good idea in itself? I'd say "yes", although I can also see the argument that more local errors would be reported on a filename which is the temporary file (if a preprocessor outputs a source file -- without line directives -- not a marshaled Parsetree), and it might not be very coherent to treat global errors differently. So I'm not sure.

In the case of both ocamldep and ocamlopt, Location.input_name is explicitly set before any Pparse function is called. If we set it in preprocess, that means that we could override this value in a way that is wrong (for example if the input was copied to a temporary _build directory first?).

I am not sure how other tools handle this. I couldn't find any code calling Pparse.file on github.

(I am skeptical about the "more local errors" argument: in all cases I have interacted with, talking about intermediate files they don't know about (especially when they have random names, but _build/foo was also an issue) was perceived as a usability bug by users. If the tool use Pparse is done well, it should report its errors on the post-processed file using error positions that refer to the source code, so mentioning the source path is the way to go.)

If the tool use Pparse is done well, it should report its errors on the post-processed file using error positions that refer to the source code, so mentioning the source path is the way to go.)

How could it do that? If a preprocessor outputs a source file without line directives, the "driver" tools has no way to map an error in this generated file back to a location in the original unprocessed file. At least if we show the temporary file and keep this file around, one can see what the error actually is. (It could also be useful to show the original filename as well.)

If you are worry about tools calling the wrong functions, maybe they should be documented ?

In particular, I had no idea that half of Pparse existed (since it's not in parsing/), the last time I needed to read an ml file, I did it like that (and it took me some time to realize I needed to set Location.input_name to have decent error messages).

Yeah, Pparse has been nicely improved through the use of GADTs, but the API remains a bit of of under-documented mess. It would make sense to move its "modern" functionalities to a new, well-documented module under parsing/ and deprecate Pparse itself. Contributions are welcome!

@gasche Can you try to bring this pull request to a conclusion? I've asked @diml to look at this too since he has a better idea of what might break. (Not that compiler-libs has any stability guarantees, IIUC.)

My gut feeling is that "making it easier to get reproducible builds of OCaml project" is useful and offsets the pain of asking unknown Pparse-using OCaml tools to set their input locations explicitly.