incremental: hash items' source tokens to generate DefId's.

[eddyb]

We should strive to handle the "only touched one item" case very well, which means either knowing the exact modified range, or inferring it through a series of heuristics.

The current DefPath machinery doesn't know how to properly understand things like e.g. impls haven't changed, and would assume that adding an impl invalidates later ones in the same module.

If we only apply the existing machinery between the "leftover" old and new items that didn't have a match in their "source tokens hash", we should be able to get overall more consistent DefId matches, even in trickier situations where, for example, the whole file was moved around.

Original source could also be used, but ignoring whitespace & comments seems useful.

---

One way I can see us proceeding is by gathering some statistics of the hit/miss rate of this scheme for typical editing scenarios, and the cost of actually doing the hashing.

We can probably compute the hash for each Item after finishing parsing it, from its tokens field (which is there for proc macros, but it should be fine to reuse for our purposes).

cc @nikomatsakis @michaelwoerister

[oli-obk]

Wouldn't this cause two token identical items in different modules to have the same DefId?

[eddyb]

@oli-obk It wouldn't be that direct. You can basically ignore such items until you've guessed their respective modules, which should narrow it down.

[eddyb]

Another experiment idea: just... don't hash identifiers (maybe only keywords?). This would give you some stability across renames.

[nikomatsakis]

So, the existing scheme can handle (i.e., won't do the wrong thing, though it may be less efficient) two items with the same tokens. You could imagine using the names if the user gave them us, and for anonymous things like impls, using this hash as the name. This would mean that if you have two impls that are token-by-token identical, they get distinct def-ids just as they do today, but otherwise it works out.

(One downside is that assigning def-ids remains a serial process.)

[nikomatsakis]

In any case, I love the idea of experimenting here.

[nikomatsakis]

Well, let me rephrase. I like the idea of experimenting, but I am dubious that this addresses any of our most urgent problems. I feel like most "incremental-style" changes are mutating the bodies of methods and things, not adding new items etc?

It'd be nice to see if we can get some kind of corpus on this.

This will definitely be true in an IDE scenario.

[steveklabnik]

Triage: no idea what the status of this issue is or if it's useful.

[nikomatsakis]

I think the answer this is still an interesting idea, still not the most urgent problem.

[pierwill]

I'd like to look into this. @rustbot claim

[michaelwoerister]

In my estimation this would be a rather fundamental change with correctness implications that are hard to predict. I also don't see how it would fundamentally improve performance. As a reference point, we recently fixed the hashing of modules to take item order into account (for correctness reasons) and that basically had no negative effect on the benchmarks, IIRC. With current, already hard to maintain state of incr. comp. I personally would not recommend changing something here without a very thorough review of the approach.

[pierwill]

Thanks, @michaelwoerister. Since I'm not in a position to do such a review, let's leave this for now. @rustbot release-assignment