incr.comp.: Update fingerprint-based auto tests for red/green tracking.

[michaelwoerister]

The incr. comp. system computes fingerprints (hashes) of various things and then uses these fingerprints to check if something has changed in comparison to the previous compilation session. The test cases in src/test/incremental/hashes test, at a fine-grained level, that various changes in the source code lead to changed fingerprints of various intermediate results.
Before red/green change tracking (implemented in #44901) the compiler only computed fingerprints for the inputs of a program (i.e. the HIR) and the things exported to crate metadata. With the new tracking system we also compute hashes for almost all intermediate results, but the test cases in src/test/incremental/hashes do not reflect that yet.

A given item is tested by attaching a #[rustc_clean] attribute to them for expressing the expectation that the item's fingerprint has not changed, or by attaching a #[rustc_dirty] attribute if the fingerprint should have changed. These attributes have two arguments:

• the label determines which kind of hash we are interested in (i.e. the DepNode), and
• the cfg argument says in which compilation session this assumption should be tested.

So, for example, if we want to assert that the fingerprint of the optimized MIR of a given function foo has changed between the first and the second compilation session and has not changed between sessions 2 and 3, we can do so as follows:

#[cfg(cfail1)]
fn foo() {
    // ...
}

#[rustc_dirty(label="MirOptimized", cfg="cfail2")]
#[rustc_clean(label="MirOptimized", cfg="cfail3")]
#[cfg(not(cfail1))]
fn foo() {
    // ...
}

Since #45104 we also have a more concise way of expressing these assertions. The following snippet of code tests that all relevant results are clean, except for "MirOptimized". This obviates the need to exhaustively list all DepKinds that should be checked. The testing framework knows which are relevant for the item the #[rustc_clean] or #[rustc_dirty] attribute is attached to.

#[cfg(cfail1)]
fn foo() {
    // ...
}

#[rustc_clean(except="MirOptimized", cfg="cfail2")]
#[rustc_clean(cfg="cfail3")]
#[cfg(not(cfail1))]
fn foo() {
    // ...
}

The #[cfg] attributes attached to the function specify which version gets compiled in which compilation session (see also #36674 for another description of how these tests work). The possible values for the label argument are those DepNode variants that have a single DefId argument.

At the time of writing, there are 134 kinds of dependency nodes and it would be overkill to test fingerprints for all of these. But, depending on the kind of item under test, there are a few key ones that we should verify:

Free-standing Functions and Methods

These represent executable code, so we want to test their MIR:

• MirValidated
• MirOptimized

Callers will depend on the signature of these items, so we better test

• TypeOfItem,
• GenericsOfItem,
• PredicatesOfItem, and
• FnSignature.

And a big part of compilation (that we eventually want to cache) is type inference information:

• TypeckTables

For methods, we can also check

• AssociatedItems

which is a bit misnamed and actually describes the ty::AssociatedItem descriptor of the method.

Struct, Enum, and Union Definitions

For these we should at least test

• TypeOfItem,
• GenericsOfItem, and
• PredicatesOfItem.

in addition to Hir and HirBody. Note that changing the type of a
field does not change the type of the struct or enum, but adding/removing
fields or changing a fields name or visibility does.

Struct/Enum/Unions Fields

Fields are kind of separate from their containers, as they can change independently from them. We should at least check

• TypeOfItem for these.

Trait Definitions

For these we'll want to check

• TraitDefOfItem,
• TraitImpls,
• SpecializationGraph,
• ObjectSafety,
• AssociatedItemDefIds,
• GenericsOfItem, and
• PredicatesOfItem

(Trait) Impls

For impls we'll want to check

• ImplTraitRef,
• AssociatedItemDefIds, and
• GenericsOfItem.

Associated Items

For associated items (types, constants, and methods) we should check

• TraitOfItem,
• AssociatedItems.

Test Files to Update

The existing tests can be found in the src/test/incremental/hashes directory. A description of how the tests were setup initially can be found in issue #36674. The basic testing strategy should stay the same -- the goal here is to add the #[rustc_dirty]/#[rustc_clean] attributes for the labels listed above. The test suite can be executed by running ./x.py test --stage 1 src/test/incremental.

If you come a across an instance where you are not sure if it should be dirty or clean, or the compiler produces a result that's different from your expectation, feel free to leave a comment below or ask on gitter or IRC.

If you want to take on updating a specific test file, leave a comment below and I'll mark it has taken.

• call_expressions.rs
• closure_expressions.rs
• consts.rs
• enum_constructors.rs
• enum_defs.rs
• exported_vs_not.rs
• extern_mods.rs
• for_loops.rs
• function_interfaces.rs
• if_expressions.rs
• indexing_expressions.rs
• inherent_impls.rs
• inline_asm.rs
• let_expressions.rs
• loop_expressions.rs
• match_expressions.rs
• panic_exprs.rs
• panic_exprs_no_overflow_checks.rs
• statics.rs
• struct_constructors.rs
• struct_defs.rs
• trait_defs.rs
• trait_impls.rs
• type_defs.rs
• unary_and_binary_exprs.rs
• while_let_loops.rs
• while_loops.rs

Good luck! :)

[vitiral]

Review comments:

• "A given item can is tested by" -> "A given item is tested by"
• #[cfg(rpass1)]: I like the new name much better than "cfail". I assume the issues of Prefixing a revision with "cfail" tells the test runner to treat the test like a "compile-file" test have somehow been resolved?
• DepNodes: I think this is a better link. Keeping the link to the implementation for the "134 kinds of dependency nodes" is also useful.
  • unrelated question on DepNode, it says the value of the fingerprint does not depend on anything that is specific to a given compilation session, like an unpredictable interning key (e.g. NodeId, DefId, Symbol)... -- I thought NodeId and DefId were both predictable? Wouldn't DefId have to be, since it is the input to most of the DepNodes?
• "it would be overkill to test fingerprints for all of these.": I assume they will all be tested as part of end-to-end tests, even if they are not each individually tested in unit tests -- correct? I think it's a bad idea to rely on crater-runs to validate these.
  • related note: it looks like a lot of thought was given to the higest risk items to test, good job!

Final comment: if I clone the compiler and build, do these currently run?

[gaurikholkar-zz]

Taking up let_expressions test

[michaelwoerister]

Thanks @gaurikholkar!

[eulerdisk]

I did the enum_defs.rs last time, so I can take that one to begin with.

[michaelwoerister]

@vitiral:

#[cfg(rpass1)]: I like the new name much better than "cfail". I assume the issues of Prefixing a revision with "cfail" tells the test runner to treat the test like a "compile-file" test have somehow been resolved?

I just changed it to cfail because it looks like the tests are using that currently. The rpass prefix is possible to and it tells the test runner that compilation and execution of the test file have to succeed. Since we are not interested in execution results here, cfail is the more efficient option. Sorry you like rpass more :)

unrelated question on DepNode, it says the value of the fingerprint does not depend on anything that is specific to a given compilation session, like an unpredictable interning key (e.g. NodeId, DefId, Symbol)... -- I thought NodeId and DefId were both predictable? Wouldn't DefId have to be, since it is the input to most of the DepNodes?

NodeId and DefId are both predictable in the sense that we will generate the same values for them when compiling exactly the same code base twice. But there is no guarantee for them that the same item (e.g. struct definition or functions, etc) will be assigned the same NodeId and/or DefId in two sessions with code changes in between. For that we have the DefPath (which derived from the actual path of an item, like in std::collections::HashMap) and the DefPathHash, which is a 128-bit hash of a DefPath. DefPath and DefPathHash are equivalent, so we are mostly using DefPathHash (unless we need something human-readable).

"it would be overkill to test fingerprints for all of these.": I assume they will all be tested as part of end-to-end tests, even if they are not each individually tested in unit tests -- correct? I think it's a bad idea to rely on crater-runs to validate these.

Well, maybe we actually should test even some more combinations. And we certainly want to cover examples for common kinds of modifications in the end-to-end tests, yes. And any problem that we find through our real-world tests and bug reports should also be turned into an appropriate regression test.

[michaelwoerister]

Thanks, @eulerdisk!

[michaelwoerister]

@vitiral

Final comment: if I clone the compiler and build, do these currently run?

Yes, running ./x.py test --stage 1 src/test/incremental in the root directory of your compiler clone will run them.

[vitiral]

@michaelwoerister thanks!

[gaurikholkar-zz]

@michaelwoerister left a comment in the gitter chat group. Thanks

[vitiral]

@michaelwoerister I'll start off with struct_defs.rs

[vitiral]

@michaelwoerister I'll take call_expressions.rs as part of the next stage in #45009

[vitiral]

#45104 is the implemenation of call_expressions.rs

[vitiral]

@michaelwoerister as part of #45104 I'll take:

• consts.rs
• enum_constructors.rs
• extern_mods.rs
• inherent_impls.rs
• statics.rs
• struct_constructors.rs
• trait_defs.rs
• trait_impls.rs
• type_defs.rs

Edit: nevermind about closure_expressions.rs

I'm not going to leave a lot for anybody else ☹️

I don't forsee this taking too long with the new infrastructure. I think I can actually pretty much auto-write the tests with a sed expression and then the compiler will tell me what to exclude (which I will manually verify as making sense and ask you a bunch of stupid questions I'm sure 😉)

Edit: @michaelwoerister is taking trait_defs + trait_impls since I was blocked by an ICE on them.

[michaelwoerister]

@vitiral Go for it :)

[gaurikholkar-zz]

I'll take up match_expressions.rs too

[eulerdisk]

@vitiral I was waiting for your work on except to land before to start but If you are on a roll you can take "my" enum_defs.rs too if you want. You are certainly more productive than me at this point 😄

[vitiral]

@eulerdisk I actually really wanted to do enum_defs, so I'll take it!

[vitiral]

@eulerdisk done.

I'm going to be on haitus for a bit after this gets merged, so you can pick up my slack 😄

[vitiral]

@michaelwoerister with #45104 merged, the tutorial in this issue should probably be updated!

[michaelwoerister]

@vitiral Yes, that's a good idea. Will do so soon.

[michaelwoerister]

@vitiral Updated.

[CrockAgile]

@michaelwoerister as a first step I'll take:

• exported_vs_not.rs
• if_expressions.rs
• panic_exprs.rs
• panic_exprs_no_overflow_checks.rs

Some of these tests are encountering missing fingerprints (Could not find current fingerprint for MirOptimized):

• closure_expressions.rs
• inline_asm.rs
• indexing_expressions.rs
• function_interfaces.rs
• while_loops.rs

[fitzgen]

I'm hacking on unary_and_binary_exprs.rs.

[CrockAgile]

@michaelwoerister hopefully it isn't overly greedy of me but since there hasn't been any updates for a bit, I had an urge to finish off the checklist. see #46523

[michaelwoerister]

With @CrockAgile's final push we have now updated all tests. Thank you everyone!

[vitiral]

@michaelwoerister it looks like trait-impls is still outstanding with the bug I hit.

I forgot to mention, but I took a stab at it a month ago with your "simple fix" and was not successful (still hitting ICE). You might want to look into it further?