Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

incr.comp.: Create Test Case for Incr. Comp. Hash for unary and binary expressions #37520

Closed
michaelwoerister opened this issue Nov 1, 2016 · 3 comments
Labels
A-incr-comp Area: Incremental compilation A-testsuite Area: The testsuite used to check the correctness of rustc E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion.

Comments

@michaelwoerister
Copy link
Member

This issue is part of a broader effort to implement comprehensive regression testing for incremental compilation. For the tracking issue go to #36350.

Background

For incremental compilation we need to determine if a given HIR node has changed in between two versions of a program. This is implemented in the calculate_svh module. We compute a hash value for each HIR node that corresponds to a node in the dependency graph and then compare those hash values. We call this hash value the Incremental Compilation Hash (ICH) of the HIR node. It is supposed to be sensitive to any change that might make a semantic difference to the thing being hashed.

Testing Methodology

The auto-tests in the src/test/incremental directory all follow the same pattern:

  • Each source file defines one test case
  • The source file is compiled multiple times with incremental compilation turned on, each time with a different --cfg flag, allowing each version to differ via conditional compilation. Each of these versions we call a revision.
  • During each revision, the test runner will make sure that some assertions about the dependency graph of the program hold.
  • These assertions are specified in the source code of the test file as attributes attached to the items being tested (e.g. #[rustc_clean]/#[rustc_dirty]).

The ICH-specific tests use this framework by adhering to the following pattern:

  • There are two versions of the definition under test, one marked with #[cfg(cfail1)] and the second marked with #[cfg(not(cfail1))]. As a consequence, the first revision will be compiled with the first version of the definition, and all other revisions will be compiled with the second version. The two versions are supposed to differ in exactly one aspect (e.g. the visibility of a field is different, or the return of a function has changed).
  • The second definition has a #[rustc_dirty(label="Hir", cfg="cfail2")] attribute attached. This attribute makes the test runner check that a change of the Hir dependency node of the definition has been detected between revisions cfail1 and cfail2. This will effectively test that a different ICH value has been computed for the two versions of the definition.
  • The second definition also has a #[rustc_clean(label="Hir", cfg="cfail3")] attribute. This makes sure that the Hir dependency node (and thus the ICH) of the definition has not changed between revisions cfail2 and cfail3. That's what we expect, because both revisions use the same version of the definition.
  • For definitions that are exported from the crate, we also want to check the ICH of the corresponding metadata. This is tested using the #[rustc_metadata_clean]/#[rustc_metadata_dirty] attributes and works analogous to the Hir case: We add #[rustc_metadata_dirty(cfg="cfail2")] to the second definition to make sure that the ICH of the exported metadata is not the same for the different versions of the definition, and we add #[rustc_metadata_dirty(cfg="cfail3")] to make sure that the ICH is the same for the two identical versions of the definition.

Why are the revisions called "cfail"? That's because of reasons internal to how
the test-runner works. Prefixing a revision with "cfail" tells the test runner to treat the test like a "compile-file" test, that is: compile the test case but don't actually run it (which would be the case for an "rpass" revision). For the ICH tests we need to compile "rlibs", so that we can test metadata ICHs. As a consequence we cannot declare them "rpass". In an additional directive (// must-compile-successfully), we tell the test runner that we actually expect the file to not produce any errors.

Each test case must contain the following test-runner directives and crate attributes at the top:

// must-compile-successfully
// revisions: cfail1 cfail2 cfail3
// compile-flags: -Z query-dep-graph

#![feature(rustc_attrs)]  // <-- let's us use #[rustc_dirty], etc.
#![crate_type="rlib"]     // <-- makes sure that we export definitions

See src/test/incremental/hashes/struct_defs.rs for a full example of such a ICH regression test.

Unary and Binary Expression Specific ICH Testing

Each of the following things should be tested with its own definition pair:

  • Change constant operand of negation (e.g. -10 to -1)
  • Change constant operand of bitwise not (e.g. !100 to !99)
  • Change variable operand of negation (e.g. -x to -y)
  • Change variable operand of bitwise not (e.g. !x to !y)
  • Change variable operand of deref (e.g. *x to *y)
  • Change first constant operand of addition (e.g. 1 + 3 to 2 + 3)
  • Change second constant operand of addition (e.g. 1 + 2 to 1 + 3)
  • Change first variable operand of addition (e.g. a + 2 to b + 3)
  • Change second variable operand of addition (e.g. 1 + a to 1 + b)
  • Change operator from + to - (e.g. 1 + a to 1 - a)
  • Change operator from + to * (e.g. 1 + a to 1 * a)
  • Change operator from + to / (e.g. 1 + a to 1 / a)
  • Change operator from + to % (e.g. 1 + a to 1 % a)
  • Change operator from && to || (e.g. a && b to a || b)
  • Change operator from & to | (e.g. 1 & a to 1 | a)
  • Change operator from & to ^ (e.g. 1 & a to 1 ^ a)
  • Change operator from & to << (e.g. a & 1 to a << 1)
  • Change operator from & to >> (e.g. a & 1 to a >> 1)
  • Change operator from == to != (e.g. a == 1 to a != 1)
  • Change operator from == to < (e.g. a == 1 to a < 1)
  • Change operator from == to > (e.g. a == 1 to a > 1)
  • Change operator from == to <= (e.g. a == 1 to a <= 1)
  • Change operator from == to >= (e.g. a == 1 to a >= 1)
  • Change type in cast expression (e.g. x as u32 to x as i32)
  • Change value in cast expression (e.g. x as u32 to y as i32)
  • Change l-value in assignment (e.g. x =10 to x = 11)
  • Change r-value in assignment (e.g. x =123 to y = 123)
  • Change index into slice (e.g. s[i] to s[j])

Note that this test case should contain -Z force-overflow-checks=off in its compile-flags, since that influences the generated hash for some of these expressions.

@michaelwoerister michaelwoerister added E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. A-testsuite Area: The testsuite used to check the correctness of rustc E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. A-incr-comp Area: Incremental compilation labels Nov 1, 2016
@zimurgh
Copy link

zimurgh commented Nov 1, 2016

I'd be interested in taking on this one!

@michaelwoerister
Copy link
Member Author

@OldManMike Excellent! Let me know if you have any questions.

As the post says, taking src/test/incremental/hashes/struct_defs.rs as a starting point is a good idea. One thing to keep an eye on is that the change in each of the test cases should really be the only change being made. So, for example, if you need to change the variable being used from x to y, then y should already be present in the initial version, even though it might not be used there.

steveklabnik added a commit to steveklabnik/rust that referenced this issue Nov 8, 2016
…r=michaelwoerister

Add unary and binary tests for incr-comp

This is my draft of tests for unary and binary expressions as desired by rust-lang#37520 for use in the test suite for hashes in incremental compilation. Feedback would be wonderful, if there's any changes I need to make I would appreciate the code review.

?r @michaelwoerister
@michaelwoerister
Copy link
Member Author

This is solved by #37610. Thanks, @OldManMike!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-incr-comp Area: Incremental compilation A-testsuite Area: The testsuite used to check the correctness of rustc E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion.
Projects
None yet
Development

No branches or pull requests

2 participants