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

Error generation: add tests and fix Naive variant #93

Merged
merged 3 commits into from Dec 7, 2018

Conversation

lqd
Copy link
Member

@lqd lqd commented Dec 6, 2018

This PR:

  • adds the facts from rustc's polonius_smoke_test: it's a dataset containing both positive and negative cases for polonius, making it easy to test error generation both in unit tests and manually.
  • adds missing invalidates inputs to the Naive variant, to successfully generate errors.

r? @nikomatsakis

This is a dataset generating polonius errors in all variants.
2 of the smoke test functions pass the analysis, 4 functions don't.
The `invalidates` facts being empty prevented the variant from generating errors.
@lqd lqd mentioned this pull request Dec 6, 2018
@lqd
Copy link
Member Author

lqd commented Dec 7, 2018

Maybe I should have added the successful tests to the tests! list ?

(I hadn't done that in order to add assertions about the LocationInsensitive variant for those 2 tests, but maybe they're not that important — and if so, adding them using the macro instead would reduce code duplication)

@@ -207,3 +208,76 @@ fn borrowed_local_error() {
let facts = parse_from_program(program, &mut tables).expect("Parsing failure");
test_facts(&facts, Algorithm::OPTIMIZED);
}

#[test]
fn smoke_test_errors() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ugh. good to test, but we gotta make this more concise.

@@ -79,6 +79,9 @@ pub(super) fn compute<Region: Atom, Loan: Atom, Point: Atom>(
// load initial facts.
subset.insert(all_facts.outlives.into());
requires.insert(all_facts.borrow_region.into());
invalidates.insert(Relation::from(
all_facts.invalidates.iter().map(|&(p, b)| ((b, p), ())),
));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

....huh. Whoops.

@nikomatsakis nikomatsakis merged commit cd82bbc into rust-lang:master Dec 7, 2018
@lqd lqd deleted the factchecking branch December 7, 2018 11:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants