Refactor typedtree traverse to use compiler traverse

[panglesd]

This is hopefully less maintenance, increased compatibility, shorter code, and the guaranty that we don't miss any occurrence of some deeply nested core type (for instance) for source features such as #976.

[Julow]

Looks good! The code indeed looks much more maintainable.

I couldn't find new links but I noticed that this now generates more anchors (for example in structures passed as argument to functors).

[panglesd]

I noticed that this now generates more anchors

Could you give me an example? In particular, is the new anchor a "local anchor", or a "global anchor"?
I expect it to be a local anchors. The global anchors should be the same, but it is very possible that the old manual traverse was missing local anchors.

[Julow]

By diffing the output of @docgen, I only see changes in local anchors. For example, in src/document/ML.ml there's new local anchors for close_tag_closed, etc..
Here's the observed diff: https://gist.github.com/Julow/230e1e240453137f8cbcc5072e75c47a

[panglesd]

Thanks. Indeed, two anchors for the same identifier are generated... let me investigate!

[panglesd]

The post-processing of source info is now more careful to not add two definition at the same place. In case of "conflicts", we use the "local anchor", which is more readable, rather than the def_%d anchor.

[gpetiot]

It looks good to me!

[Julow]

Looks good :)
I love the new local_... anchors! Though, the diff is now too large to be inspected, I believe the code to be correct.

[panglesd]

While trying to add a comment as suggested in #1031 (comment), I realized that the code was not clear enough...

In 48f8f06 I tried to improve that. @Julow is it clearer like that?

[jonludlam]

The post-processing of source info is now more careful to not add two definition at the same place. In case of "conflicts", we use the "local anchor", which is more readable, rather than the def_%d anchor.

Just want to check that that's not going to lead to broken links? As in, if something is linking here and we've dropped the def_... anchor in favour of the local one, won't that break the link?

[panglesd]

Just want to check that that's not going to lead to broken links? As in, if something is linking here and we've dropped the def_... anchor in favour of the local one, won't that break the link?

No, that's not going to lead to any dead link. All source ids are stored in a loc to id table, from which we generate both the links and the anchors.

[jonludlam]

In it goes!