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

tracking issue for rustc_dirty/clean enhancements #45009

Closed
2 of 3 tasks
vitiral opened this issue Oct 3, 2017 · 4 comments · Fixed by #85331
Closed
2 of 3 tasks

tracking issue for rustc_dirty/clean enhancements #45009

vitiral opened this issue Oct 3, 2017 · 4 comments · Fixed by #85331
Labels
A-incr-comp Area: Incremental compilation A-testsuite Area: The testsuite used to check the correctness of rustc C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC E-help-wanted Call for participation: Help is requested to fix this issue. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@vitiral
Copy link
Contributor

vitiral commented Oct 3, 2017

cc @michaelwoerister

this is partof #44924

I started this work in #44983, but I would like to break it out into three separate pull requests.

This feature will make it so that rustc_clean(except="foo,bar") will assert that ALL DefNode's are clean (which is different depending on which def-node it is) except for the ones given in except= (which it asserts are dirty)

Example syntax:

#[rustc_clean(except="Hir,HirBody,TypeOfItem", cfg="cfail2")]
#[rustc_clean(cfg="cfail3")]
struct Abc { ... }

Instead of how it is currently done:

#[rustc_dirty(label="Hir", cfg="cfail2")]
#[rustc_dirty(label="HirBody", cfg="cfail2")]
#[rustc_dirty(label="TypeOfItem", cfg="cfail2")]
#[rustc_clean(label="GenericsOfItem", cfg="cfail2")]
#[rustc_clean(label="PredicatesOfItem", cfg="cfail2")]
#[rustc_clean(label="Hir", cfg="cfail3")]
#[rustc_clean(label="HirBody", cfg="cfail3")]
#[rustc_clean(label="TypeOfItem", cfg="cfail3")]
#[rustc_clean(label="GenericsOfItem", cfg="cfail3")]
#[rustc_clean(label="PredicatesOfItem", cfg="cfail3")]
struct Abc { ... }

This should make testing incr-compilation more complete, less boilerplate and easier to read (but also an itty bit more magic)

This work is split into three parts:

  1. groundwork for rustc_clean/dirty improvements #44983: allow multiple nodes in label= to support that functionality in except in the future. This requires a couple of changes to librustc, so I want to merge it separately.
  2. Incremental compilation auto assert (with except) #45104 add except= and detect the DefNode and do correct assertions. Also use it in a couple of test files (continue to support label=)
  3. depricate label= and remove from all tests.

Item 1 adds rustc::dep_graph::dep_node::label_strs so that we can specify groups in this way:

const STRUCT_NODES: &'static str = [
    label_strs.Hir,
    label_strs.HirBody,
    label_strs.TypeOfItem,
    label_strs.GenericsOfItem
    label_strs.PredicatesOfItem
];
@TimNN TimNN added A-incr-comp Area: Incremental compilation C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC labels Oct 3, 2017
@michaelwoerister
Copy link
Member

@vitiral Sounds like good plan to me!

vitiral added a commit to vitiral/rust that referenced this issue Oct 4, 2017
bors added a commit that referenced this issue Oct 14, 2017
Incremental compilation auto assert (with except)

cc @michaelwoerister

bors merged part 1, so this is a WIP of part 2 of #45009  -- auto asserting DepNodes depending on the type of node rustc_clean/dirty is attached to

Framework:
- [x] finish auto-detection for specified DepNodes
- [x] finish auto-detection for remaining DepNodes

Test Refactors:
- [x] consts.rs
- [x] enum_constructors.rs
- [x] extern_mods.rs
- [x] inherent_impls.rs
- [x] statics.rs
- [x] struct_constructors.rs
- ~~**BLOCKED** trait_defs.rs, see FIXME~~
- ~~**BLOCKED** trait_impls.rs~~
- [x] type_defs.rs
- [x] enum_defs.rs
@pnkfelix pnkfelix added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Mar 29, 2019
@jonas-schievink
Copy link
Contributor

Triage: The only unchecked box is "depricate label= and remove from all tests."

I'm not sure if that has happened. Is this even still a useful thing to do?

@jonas-schievink jonas-schievink added the A-testsuite Area: The testsuite used to check the correctness of rustc label Nov 26, 2019
@vitiral
Copy link
Contributor Author

vitiral commented Nov 26, 2019

probably not required, but I generally feel it's better to have only one way to do things :D

@jonas-schievink jonas-schievink added the E-help-wanted Call for participation: Help is requested to fix this issue. label Nov 26, 2019
@Aaron1011
Copy link
Member

It looks like the current approach relies on a hardcoded list of DepNodes for each attribute target. Is there any reason why we don't just check all DepNodes in existence for a given target?

@bors bors closed this as completed in 153f22a Jun 2, 2021
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 C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC E-help-wanted Call for participation: Help is requested to fix this issue. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants