Setup two tasks, one of which only processes the signatures, in order to
isolate the typeck entries for signatures from those for bodies.
Fixes#36078Fixes#37720
This used to work with the rustc_clean attribute, but doesn't anymore
since my rebase; but I don't know enough about the type checking to find
out what's wrong. The dep graph looks like this:
ItemSignature(xxxx) -> CollectItem(xxxx)
CollectItem(xxxx) -> ItemSignature(xxxx)
ItemSignature(xxxx) -> TypeckItemBody(yyyy)
HirBody(xxxx) -> CollectItem(xxxx)
The cycle between CollectItem and ItemSignature looks wrong, and my
guess is the CollectItem -> ItemSignature edge shouldn't be there, but
I'm not sure how to prevent it.
Separate impl items from the parent impl
This change separates impl item bodies out of the impl itself. This gives incremental more resolution. In so doing, it refactors how the visitors work, and cleans up a bit of the collect/check logic (mostly by moving things out of collect that didn't really belong there, because they were just checking conditions).
However, this is not as effective as I expected, for a kind of frustrating reason. In particular, when invoking `foo.bar()` you still wind up with dependencies on private items. The problem is that the method resolution code scans that list for methods with the name `bar` -- and this winds up touching *all* the methods, even private ones.
I can imagine two obvious ways to fix this:
- separating fn bodies from fn sigs (#35078, currently being pursued by @flodiebold)
- a more aggressive model of incremental that @michaelwoerister has been advocating, in which we hash the intermediate results (e.g., the outputs of collect) so that we can see that the intermediate result hasn't changed, even if a particular impl item has changed.
So all in all I'm not quite sure whether to land this or not. =) It still seems like it has to be a win in some cases, but not with the test cases we have just now. I can try to gin up some test cases, but I'm not sure if they will be totally realistic. On the other hand, some of the early refactorings to the visitor trait seem worthwhile to me regardless.
cc #36349 -- well, this is basically a fix for that issue, I guess
r? @michaelwoerister
NB: Based atop of @eddyb's PR https://github.com/rust-lang/rust/pull/37402; don't land until that lands.
Before, when we created an AssociatedItem for impl item X, we would read
the impl item itself. Now we instead load up the impl I that contains X
and read the data from the `ImplItemRef` for X; actually, we do it for
all impl items in I pre-emptively.
This kills the last source of edges between a method X and a call to a
method Y defined in the same impl.
Fixes#37121
ICH: Hash expression spans if their source location is captured for panics.
Since the location of some expressions is captured in error message constants, it has an influence on machine code and consequently we need to take them into account by the incr. comp. hash. This PR makes this happen for `+, -, *, /, %` and for array indexing -- let me know if I forgot anything.
In the future we might want to change the codegen strategy for those error messages, so that they are stored in a separate object file with a stable symbol name, so that only this object file has to be regenerated when source locations change. This strategy would also eliminate unnecessary duplications due to monomorphization, as @arielb1 has pointed out on IRC. I opened https://github.com/rust-lang/rust/issues/37512, so we don't forget about this.
r? @nikomatsakis
add more incremental reuse test cases
r? @michaelwoerister
This is basically a port of the "private method in impl". It works better when it's a top-level fn. =)
