From b9e35a164416272a3aa3778c2ce9669dcc15c556 Mon Sep 17 00:00:00 2001 From: Huon Wilson Date: Wed, 9 Jul 2014 22:02:19 +1000 Subject: [PATCH 1/2] lint: extend `#[must_use]` to handle a message. Similar to the stability attributes, a type annotated with `#[must_use = "informative snippet"]` will print the normal warning message along with "informative snippet". This allows the type author to provide some guidance about why the type should be used. --- src/librustc/lint/builtin.rs | 32 +++++++++++++++++--------- src/test/compile-fail/unused-result.rs | 8 +++++++ 2 files changed, 29 insertions(+), 11 deletions(-) diff --git a/src/librustc/lint/builtin.rs b/src/librustc/lint/builtin.rs index ae401b9d6f15c..481187f7c2ce7 100644 --- a/src/librustc/lint/builtin.rs +++ b/src/librustc/lint/builtin.rs @@ -669,22 +669,13 @@ impl LintPass for UnusedResult { if ast_util::is_local(did) { match cx.tcx.map.get(did.node) { ast_map::NodeItem(it) => { - if attr::contains_name(it.attrs.as_slice(), - "must_use") { - cx.span_lint(UNUSED_MUST_USE, s.span, - "unused result which must be used"); - warned = true; - } + warned |= check_must_use(cx, it.attrs.as_slice(), s.span); } _ => {} } } else { csearch::get_item_attrs(&cx.sess().cstore, did, |attrs| { - if attr::contains_name(attrs.as_slice(), "must_use") { - cx.span_lint(UNUSED_MUST_USE, s.span, - "unused result which must be used"); - warned = true; - } + warned |= check_must_use(cx, attrs.as_slice(), s.span); }); } } @@ -693,6 +684,25 @@ impl LintPass for UnusedResult { if !warned { cx.span_lint(UNUSED_RESULT, s.span, "unused result"); } + + fn check_must_use(cx: &Context, attrs: &[ast::Attribute], sp: Span) -> bool { + for attr in attrs.iter() { + if attr.check_name("must_use") { + let mut msg = "unused result which must be used".to_string(); + // check for #[must_use="..."] + match attr.value_str() { + None => {} + Some(s) => { + msg.push_str(": "); + msg.push_str(s.get()); + } + } + cx.span_lint(UNUSED_MUST_USE, sp, msg.as_slice()); + return true; + } + } + false + } } } diff --git a/src/test/compile-fail/unused-result.rs b/src/test/compile-fail/unused-result.rs index 44058c1ddda19..ecc52c0ee7d58 100644 --- a/src/test/compile-fail/unused-result.rs +++ b/src/test/compile-fail/unused-result.rs @@ -14,27 +14,35 @@ #[must_use] enum MustUse { Test } +#[must_use = "some message"] +enum MustUseMsg { Test2 } + fn foo() -> T { fail!() } fn bar() -> int { return foo::(); } fn baz() -> MustUse { return foo::(); } +fn qux() -> MustUseMsg { return foo::(); } #[allow(unused_result)] fn test() { foo::(); foo::(); //~ ERROR: unused result which must be used + foo::(); //~ ERROR: unused result which must be used: some message } #[allow(unused_result, unused_must_use)] fn test2() { foo::(); foo::(); + foo::(); } fn main() { foo::(); //~ ERROR: unused result foo::(); //~ ERROR: unused result which must be used + foo::(); //~ ERROR: unused result which must be used: some message let _ = foo::(); let _ = foo::(); + let _ = foo::(); } From 27d18fbe41665eef798eb29660c1883bb8600502 Mon Sep 17 00:00:00 2001 From: Huon Wilson Date: Wed, 9 Jul 2014 22:13:14 +1000 Subject: [PATCH 2/2] core: add `#[must_use]` attributes to iterator adaptor structs. It can be a little unintuitive that something like `v.iter().map(|x| println!("{}", x));` does nothing: the majority of the iterator adaptors are lazy and do not execute anything until something calls `next`, e.g. a `for` loop, `collect`, `fold`, etc. The majority of such errors can be seen by someone writing something like the above, i.e. just calling an iterator adaptor and doing nothing with it (and doing this is certainly useless), so we can co-opt the `must_use` lint, using the message functionality to give a hint to the reason why. Fixes #14666. --- src/libcore/iter.rs | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/src/libcore/iter.rs b/src/libcore/iter.rs index 5895d871dbe18..737ad0ae91362 100644 --- a/src/libcore/iter.rs +++ b/src/libcore/iter.rs @@ -750,6 +750,7 @@ impl, U: ExactSize> ExactSize<(A, B)> for Zip {} /// An double-ended iterator with the direction inverted #[deriving(Clone)] +#[must_use = "iterator adaptors are lazy and do nothing unless consumed"] pub struct Rev { iter: T } @@ -778,6 +779,7 @@ impl + RandomAccessIterator> RandomAccessIterato } /// A mutable reference to an iterator +#[must_use = "iterator adaptors are lazy and do nothing unless consumed"] pub struct ByRef<'a, T> { iter: &'a mut T } @@ -1038,6 +1040,7 @@ impl> CloneableIterator for T { /// An iterator that repeats endlessly #[deriving(Clone)] +#[must_use = "iterator adaptors are lazy and do nothing unless consumed"] pub struct Cycle { orig: T, iter: T, @@ -1089,6 +1092,7 @@ impl> RandomAccessIterator for Cycle /// An iterator which strings two iterators together #[deriving(Clone)] +#[must_use = "iterator adaptors are lazy and do nothing unless consumed"] pub struct Chain { a: T, b: U, @@ -1158,6 +1162,7 @@ for Chain { /// An iterator which iterates two other iterators simultaneously #[deriving(Clone)] +#[must_use = "iterator adaptors are lazy and do nothing unless consumed"] pub struct Zip { a: T, b: U @@ -1236,6 +1241,7 @@ RandomAccessIterator<(A, B)> for Zip { } /// An iterator which maps the values of `iter` with `f` +#[must_use = "iterator adaptors are lazy and do nothing unless consumed"] pub struct Map<'a, A, B, T> { iter: T, f: |A|: 'a -> B @@ -1286,6 +1292,7 @@ impl<'a, A, B, T: RandomAccessIterator> RandomAccessIterator for Map<'a, A } /// An iterator which filters the elements of `iter` with `predicate` +#[must_use = "iterator adaptors are lazy and do nothing unless consumed"] pub struct Filter<'a, A, T> { iter: T, predicate: |&A|: 'a -> bool @@ -1330,6 +1337,7 @@ impl<'a, A, T: DoubleEndedIterator> DoubleEndedIterator for Filter<'a, A, } /// An iterator which uses `f` to both filter and map elements from `iter` +#[must_use = "iterator adaptors are lazy and do nothing unless consumed"] pub struct FilterMap<'a, A, B, T> { iter: T, f: |A|: 'a -> Option @@ -1374,6 +1382,7 @@ for FilterMap<'a, A, B, T> { /// An iterator which yields the current count and the element during iteration #[deriving(Clone)] +#[must_use = "iterator adaptors are lazy and do nothing unless consumed"] pub struct Enumerate { iter: T, count: uint @@ -1428,6 +1437,7 @@ impl> RandomAccessIterator<(uint, A)> for Enumerat } /// An iterator with a `peek()` that returns an optional reference to the next element. +#[must_use = "iterator adaptors are lazy and do nothing unless consumed"] pub struct Peekable { iter: T, peeked: Option, @@ -1478,6 +1488,7 @@ impl<'a, A, T: Iterator> Peekable { } /// An iterator which rejects elements while `predicate` is true +#[must_use = "iterator adaptors are lazy and do nothing unless consumed"] pub struct SkipWhile<'a, A, T> { iter: T, flag: bool, @@ -1516,6 +1527,7 @@ impl<'a, A, T: Iterator> Iterator for SkipWhile<'a, A, T> { } /// An iterator which only accepts elements while `predicate` is true +#[must_use = "iterator adaptors are lazy and do nothing unless consumed"] pub struct TakeWhile<'a, A, T> { iter: T, flag: bool, @@ -1551,6 +1563,7 @@ impl<'a, A, T: Iterator> Iterator for TakeWhile<'a, A, T> { /// An iterator which skips over `n` elements of `iter`. #[deriving(Clone)] +#[must_use = "iterator adaptors are lazy and do nothing unless consumed"] pub struct Skip { iter: T, n: uint @@ -1615,6 +1628,7 @@ impl> RandomAccessIterator for Skip { /// An iterator which only iterates over the first `n` iterations of `iter`. #[deriving(Clone)] +#[must_use = "iterator adaptors are lazy and do nothing unless consumed"] pub struct Take { iter: T, n: uint @@ -1664,6 +1678,7 @@ impl> RandomAccessIterator for Take { /// An iterator to maintain state while iterating another iterator +#[must_use = "iterator adaptors are lazy and do nothing unless consumed"] pub struct Scan<'a, A, B, T, St> { iter: T, f: |&mut St, A|: 'a -> Option, @@ -1688,6 +1703,7 @@ impl<'a, A, B, T: Iterator, St> Iterator for Scan<'a, A, B, T, St> { /// An iterator that maps each element to an iterator, /// and yields the elements of the produced iterators /// +#[must_use = "iterator adaptors are lazy and do nothing unless consumed"] pub struct FlatMap<'a, A, T, U> { iter: T, f: |A|: 'a -> U, @@ -1747,6 +1763,7 @@ impl<'a, /// An iterator that yields `None` forever after the underlying iterator /// yields `None` once. #[deriving(Clone)] +#[must_use = "iterator adaptors are lazy and do nothing unless consumed"] pub struct Fuse { iter: T, done: bool @@ -1819,6 +1836,7 @@ impl Fuse { /// An iterator that calls a function with a reference to each /// element before yielding it. +#[must_use = "iterator adaptors are lazy and do nothing unless consumed"] pub struct Inspect<'a, A, T> { iter: T, f: |&A|: 'a @@ -2298,4 +2316,3 @@ pub mod order { } } } -