Prepend crate name to functions with nr2

[P-E-P]

Some pre nr2 were relying on function name, those name are prefixed with the crate's name.

[powerboat9]

I'm pretty sure this is an issue with how canonical paths are generated -- it should probably be fixed in ForeverStack::to_canonical_path

[P-E-P]

I'm pretty sure this is an issue with how canonical paths are generated -- it should probably be fixed in ForeverStack::to_canonical_path

I thought about this and I don't think so because we should not be able to resolve my_crate::XXXX

https://godbolt.org/z/o6zz1hfKb

The usual canonical path from within the crate is crate::XXXX, that being said I'm not entirely satisfied with my solution.

Should we provide internal_canonical_path and external_canonical_path to differentiate those two situations ?

[powerboat9]

We could have all lookups stop before they reach the parent of a ForeverStack node representing a crate -- as-is, the root ForeverStack node represents the current crate namespace, but it would probably make things easier with extern crates and preludes if crates had their own nodes. That would make lookups work correctly (all lookups would be done relative to my_crate:: or one of its descendants, baring edition-specific leading :: behavior) and make the current canonical path calculation code work properly (instead of ROOT(my_crate)->foo->bar corresponding to foo::bar, ROOT->my_crate->foo->bar would correspond to my_crate::foo::bar)

[powerboat9]

It does look like we extend canonical paths to be global between crates and to exist for most (all? virtually all?) items, where in rustc canonical paths have meaning local to a crate and only cover some items.

https://doc.rust-lang.org/reference/paths.html

We should be fine, as long as we can still map our canonical paths to rustc-like canonical paths when necessary.

[powerboat9]

Rereading ResolveTopLevel::go, it looks like two canonical paths are tracked for every item, although the one that doesn't have a prepended crate name seems to only be used inside the 1.0 resolver.

[P-E-P]

Rereading ResolveTopLevel::go, it looks like two canonical paths are tracked for every item, although the one that doesn't have a prepended crate name seems to only be used inside the 1.0 resolver.

This is probably why it was working as intended with nr1, I'll give it a look tomorrow

[CohenArthur]

I think those changes are fine. My gut feeling is we probably need to do something similar for exports? But I'm not sure. I think this is a fine solution though

[P-E-P]

Tbh I don't really like my own solution, initially I intended to modify the name resolution, add a name to the root node.

Cons for this method are obvious as name resolution from crate name should be prevented. I'll take a few days to give this PR some thoughts.

[CohenArthur]

I think the simplest option here is the best one. This is a two-line change, whereas adding a name to the root would require changes in the forever stack and name resolution overall. Name resolution is mostly needed for the current crate - exports and imports are different and can be special cased. Resolving imports will always be a lot simpler than doing name resolution on the current crate, because we can control the representation of the exported data and it will all be flattened out.

I think we should merge this, and not care about adding that little extra segment in front of the path. You can add a comment above the change to explain that we need to prepend IR names with the crate name for exporting reasons.

I don't want to change name resolution and then have to do mental gymnastics to stop at the parent node and all that. name resolution is already extremely complex and I want it to stay as simple as possible