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

RFC 1940 housekeeping #44620

Merged
merged 2 commits into from
Sep 19, 2017
Merged

RFC 1940 housekeeping #44620

merged 2 commits into from
Sep 19, 2017

Conversation

zackmdavis
Copy link
Member

r? @nikomatsakis

(It was put forward that all tests related to a feature being in their own
directory makes stabilization decisionmaking more convenient.)
@zackmdavis
Copy link
Member Author

@nikomatsakis (Or am I reading your comment too literally?—if the tests were already just one file, maybe that counts as "collected".)

@zackmdavis
Copy link
Member Author

The test diff makes more sense with looser-than-default rename detection (--find-renames=40%):

diff --git a/src/test/ui/lint/fn_must_use.rs b/src/test/ui/rfc_1940-must_use_on_functions/fn_must_use.rs
similarity index 47%
rename from src/test/ui/lint/fn_must_use.rs
rename to src/test/ui/rfc_1940-must_use_on_functions/fn_must_use.rs
index c549ded..7eb4c329 100644
--- a/src/test/ui/lint/fn_must_use.rs
+++ b/src/test/ui/rfc_1940-must_use_on_functions/fn_must_use.rs
@@ -12,7 +12,7 @@
 #![warn(unused_must_use)]
 
 struct MyStruct {
-    n: usize
+    n: usize,
 }
 
 impl MyStruct {
@@ -22,7 +22,34 @@ impl MyStruct {
     }
 }
 
-#[must_use="it's important"]
+trait EvenNature {
+    #[must_use = "no side effects"]
+    fn is_even(&self) -> bool;
+}
+
+impl EvenNature for MyStruct {
+    fn is_even(&self) -> bool {
+        self.n % 2 == 0
+    }
+}
+
+trait Replaceable {
+    fn replace(&mut self, substitute: usize) -> usize;
+}
+
+impl Replaceable for MyStruct {
+    // ↓ N.b.: `#[must_use]` attribute on a particular trait implementation
+    // method won't work; the attribute should be on the method signature in
+    // the trait's definition.
+    #[must_use]
+    fn replace(&mut self, substitute: usize) -> usize {
+        let previously = self.n;
+        self.n = substitute;
+        previously
+    }
+}
+
+#[must_use = "it's important"]
 fn need_to_use_this_value() -> bool {
     false
 }
@@ -30,6 +57,14 @@ fn need_to_use_this_value() -> bool {
 fn main() {
     need_to_use_this_value();
 
-    let m = MyStruct { n: 2 };
+    let mut m = MyStruct { n: 2 };
     m.need_to_use_this_method_value();
+    m.is_even(); // trait method!
+
+    m.replace(3);
+
+    2.eq(&3);
+
+    // FIXME: operators should probably be `must_use` if underlying method is
+    2 == 3;
 }
diff --git a/src/test/ui/lint/fn_must_use.stderr b/src/test/ui/rfc_1940-must_use_on_functions/fn_must_use.stderr
similarity index 45%
rename from src/test/ui/lint/fn_must_use.stderr
rename to src/test/ui/rfc_1940-must_use_on_functions/fn_must_use.stderr
index 2428377..69755c8 100644
--- a/src/test/ui/lint/fn_must_use.stderr
+++ b/src/test/ui/rfc_1940-must_use_on_functions/fn_must_use.stderr
@@ -1,7 +1,7 @@
 warning: unused return value of `need_to_use_this_value` which must be used: it's important
-  --> $DIR/fn_must_use.rs:31:5
+  --> $DIR/fn_must_use.rs:58:5
    |
-31 |     need_to_use_this_value();
+58 |     need_to_use_this_value();
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^
    |
 note: lint level defined here
@@ -11,8 +11,20 @@ note: lint level defined here
    |         ^^^^^^^^^^^^^^^
 
 warning: unused return value of `MyStruct::need_to_use_this_method_value` which must be used
-  --> $DIR/fn_must_use.rs:34:5
+  --> $DIR/fn_must_use.rs:61:5
    |
-34 |     m.need_to_use_this_method_value();
+61 |     m.need_to_use_this_method_value();
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
+warning: unused return value of `EvenNature::is_even` which must be used: no side effects
+  --> $DIR/fn_must_use.rs:62:5
+   |
+62 |     m.is_even(); // trait method!
+   |     ^^^^^^^^^^^^
+
+warning: unused return value of `std::cmp::PartialEq::eq` which must be used
+  --> $DIR/fn_must_use.rs:66:5
+   |
+66 |     2.eq(&3);
+   |     ^^^^^^^^^
+

@carols10cents carols10cents added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 18, 2017
@nikomatsakis
Copy link
Contributor

@bors r+

@zackmdavis this looks nice to me; I'm not that picky, but I do want to be able to easily get an overview of the current behavior, and this PR seems to enable that.

@bors
Copy link
Contributor

bors commented Sep 18, 2017

📌 Commit d09db63 has been approved by nikomatsakis

@bors
Copy link
Contributor

bors commented Sep 19, 2017

⌛ Testing commit d09db63 with merge 325ba23...

bors added a commit that referenced this pull request Sep 19, 2017
RFC 1940 housekeeping

* move test to own directory, as requested in #43302 (comment)
* exercise trait methods in test
* unstable book section

r? @nikomatsakis
@bors
Copy link
Contributor

bors commented Sep 19, 2017

☀️ Test successful - status-appveyor, status-travis
Approved by: nikomatsakis
Pushing 325ba23 to master...

@bors bors merged commit d09db63 into rust-lang:master Sep 19, 2017
@zackmdavis zackmdavis deleted the rfc_1940_housekeeping branch January 13, 2018 07:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants