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

Apply #[must_use] lint to components of tuples #61100

Merged
merged 7 commits into from Jun 3, 2019

Conversation

@varkor
Copy link
Member

commented May 23, 2019

Fixes #61061.

@rust-highfive

This comment has been minimized.

Copy link
Collaborator

commented May 23, 2019

r? @michaelwoerister

(rust_highfive has picked a reviewer for you, use r? to override)

@varkor

This comment has been minimized.

Copy link
Member Author

commented May 23, 2019

@bors try (for crater run)

@bors

This comment has been minimized.

Copy link
Contributor

commented May 23, 2019

⌛️ Trying commit 4a5e4ef with merge 49ae6b7...

bors added a commit that referenced this pull request May 23, 2019
Auto merge of #61100 - varkor:must_use-tuple-expr, r=<try>
Apply #[must_use] lint to components of tuples

Fixes #61061.
@rust-highfive

This comment was marked as resolved.

Copy link
Collaborator

commented May 23, 2019

The job x86_64-gnu-llvm-6.0 of your PR failed on Travis (raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log.
travis_time:end:0a6d3960:start=1558649915388906376,finish=1558649917764909927,duration=2376003551
$ git checkout -qf FETCH_HEAD
travis_fold:end:git.checkout

Encrypted environment variables have been removed for security reasons.
See https://docs.travis-ci.com/user/pull-requests/#pull-requests-and-security-restrictions
$ export SCCACHE_BUCKET=rust-lang-ci-sccache2
$ export SCCACHE_REGION=us-west-1
$ export GCP_CACHE_BUCKET=rust-lang-ci-cache
$ export AWS_ACCESS_KEY_ID=AKIA46X5W6CZEJZ6XT55
---
[00:48:32]    --> src/librustc_lint/lib.rs:23:9
[00:48:32]     |
[00:48:32] 23  | #![deny(internal)]
[00:48:32]     |         ^^^^^^^^
[00:48:32]     = note: #[deny(usage_of_qualified_ty)] implied by #[deny(internal)]
[00:48:32] error: aborting due to previous error
[00:48:32] 
[00:48:32] error: Could not compile `rustc_lint`.
[00:48:32] warning: build failed, waiting for other jobs to finish...
---
travis_time:end:1fb7cd40:start=1558652929905999783,finish=1558652929911379303,duration=5379520
travis_fold:end:after_failure.3
travis_fold:start:after_failure.4
travis_time:start:0c80a5bc
$ ln -s . checkout && for CORE in obj/cores/core.*; do EXE=$(echo $CORE | sed 's|obj/cores/core\.[0-9]*\.!checkout!\(.*\)|\1|;y|!|/|'); if [ -f "$EXE" ]; then printf travis_fold":start:crashlog\n\033[31;1m%s\033[0m\n" "$CORE"; gdb --batch -q -c "$CORE" "$EXE" -iex 'set auto-load off' -iex 'dir src/' -iex 'set sysroot .' -ex bt -ex q; echo travis_fold":"end:crashlog; fi; done || true
travis_fold:end:after_failure.4
travis_fold:start:after_failure.5
travis_time:start:0b74a770
travis_time:start:0b74a770
$ cat ./obj/build/x86_64-unknown-linux-gnu/native/asan/build/lib/asan/clang_rt.asan-dynamic-i386.vers || true
cat: ./obj/build/x86_64-unknown-linux-gnu/native/asan/build/lib/asan/clang_rt.asan-dynamic-i386.vers: No such file or directory
travis_fold:end:after_failure.5
travis_fold:start:after_failure.6
travis_time:start:073d1150
$ dmesg | grep -i kill

I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @TimNN. (Feature Requests)

@bors

This comment has been minimized.

Copy link
Contributor

commented May 23, 2019

💔 Test failed - checks-travis

@rust-highfive

This comment was marked as resolved.

Copy link
Collaborator

commented May 23, 2019

The job dist-x86_64-linux of your PR failed on Travis (raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log.
travis_fold:end:services

travis_fold:start:git.checkout
travis_time:start:075fb8c9
$ git clone --depth=2 --branch=try https://github.com/rust-lang/rust.git rust-lang/rust
---
[01:03:05]    --> src/librustc_lint/lib.rs:23:9
[01:03:05]     |
[01:03:05] 23  | #![deny(internal)]
[01:03:05]     |         ^^^^^^^^
[01:03:05]     = note: #[deny(usage_of_qualified_ty)] implied by #[deny(internal)]
[01:03:05] error: aborting due to previous error
[01:03:05] 
[01:03:05] error: Could not compile `rustc_lint`.
[01:03:05] warning: build failed, waiting for other jobs to finish...
---
travis_time:end:0472b7d0:start=1558653787403704513,finish=1558653787422000560,duration=18296047
travis_fold:end:after_failure.3
travis_fold:start:after_failure.4
travis_time:start:0eea0c46
$ ln -s . checkout && for CORE in obj/cores/core.*; do EXE=$(echo $CORE | sed 's|obj/cores/core\.[0-9]*\.!checkout!\(.*\)|\1|;y|!|/|'); if [ -f "$EXE" ]; then printf travis_fold":start:crashlog\n\033[31;1m%s\033[0m\n" "$CORE"; gdb --batch -q -c "$CORE" "$EXE" -iex 'set auto-load off' -iex 'dir src/' -iex 'set sysroot .' -ex bt -ex q; echo travis_fold":"end:crashlog; fi; done || true
travis_fold:end:after_failure.4
travis_fold:start:after_failure.5
travis_time:start:055e3e1c
travis_time:start:055e3e1c
$ cat ./obj/build/x86_64-unknown-linux-gnu/native/asan/build/lib/asan/clang_rt.asan-dynamic-i386.vers || true
cat: ./obj/build/x86_64-unknown-linux-gnu/native/asan/build/lib/asan/clang_rt.asan-dynamic-i386.vers: No such file or directory
travis_fold:end:after_failure.5
travis_fold:start:after_failure.6
travis_time:start:0438be74
$ dmesg | grep -i kill

I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @TimNN. (Feature Requests)

@varkor

This comment has been minimized.

Copy link
Member Author

commented May 24, 2019

@TimNN: the above error message cut off the important part.

@varkor varkor force-pushed the varkor:must_use-tuple-expr branch from 4a5e4ef to 68c0ced May 24, 2019

@varkor

This comment has been minimized.

Copy link
Member Author

commented May 24, 2019

@bors try

(I tested this before, but one of the compiler internal lints failed.)

@bors

This comment has been minimized.

Copy link
Contributor

commented May 24, 2019

⌛️ Trying commit 68c0ced with merge 58b2112...

bors added a commit that referenced this pull request May 24, 2019
Auto merge of #61100 - varkor:must_use-tuple-expr, r=<try>
Apply #[must_use] lint to components of tuples

Fixes #61061.
@bors

This comment has been minimized.

Copy link
Contributor

commented May 24, 2019

☀️ Try build successful - checks-travis
Build commit: 58b2112

@varkor

This comment has been minimized.

Copy link
Member Author

commented May 24, 2019

@craterbot run mode=check-only

@craterbot

This comment has been minimized.

Copy link
Collaborator

commented May 24, 2019

👌 Experiment pr-61100 created and queued.
🤖 Automatically detected try build 58b2112
🔍 You can check out the queue and this experiment's details.

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

@craterbot

This comment has been minimized.

Copy link
Collaborator

commented May 24, 2019

🚧 Experiment pr-61100 is now running on agent aws-3-tmp.

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

@craterbot

This comment has been minimized.

Copy link
Collaborator

commented May 26, 2019

🎉 Experiment pr-61100 is completed!
📊 2 regressed and 0 fixed (60951 total)
📰 Open the full report.

⚠️ If you notice any spurious failure please add them to the blacklist!
ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

@Centril

This comment has been minimized.

Copy link
Member

commented May 27, 2019

That's a rather small amount of regressions... Possibly too few?
...This seems bad in terms of justifying adding the lint to (t0, .., tn)?

On the other hand, the supporting code is small... thoughts?

@varkor

This comment has been minimized.

Copy link
Member Author

commented May 27, 2019

On the other hand, the supporting code is small... thoughts?

I think this change is small enough that there's not much reason to prefer not to add it if it helps catch a few more mistakes. There aren't going to be any additional false positives either.

@varkor

This comment has been minimized.

Copy link
Member Author

commented May 27, 2019

A warning is produced for each #[must_use] component of a tuple now, pointing to the correct span.

r? @cramertj

@varkor

This comment has been minimized.

Copy link
Member Author

commented Jun 2, 2019

@cramertj: this is ready for review again.

@cramertj

This comment has been minimized.

Copy link
Member

commented Jun 3, 2019

r=me with missing test. Thanks so much for doing this! :)

@varkor varkor force-pushed the varkor:must_use-tuple-expr branch from d65e7d2 to de2bf3a Jun 3, 2019

@varkor

This comment has been minimized.

Copy link
Member Author

commented Jun 3, 2019

@bors r=cramertj

@bors

This comment has been minimized.

Copy link
Contributor

commented Jun 3, 2019

📌 Commit de2bf3a has been approved by cramertj

@bors

This comment has been minimized.

Copy link
Contributor

commented Jun 3, 2019

⌛️ Testing commit de2bf3a with merge 6ffb8f5...

bors added a commit that referenced this pull request Jun 3, 2019
Auto merge of #61100 - varkor:must_use-tuple-expr, r=cramertj
Apply #[must_use] lint to components of tuples

Fixes #61061.
@bors

This comment has been minimized.

Copy link
Contributor

commented Jun 3, 2019

☀️ Test successful - checks-travis, status-appveyor
Approved by: cramertj
Pushing 6ffb8f5 to master...

@bors bors added the merged-by-bors label Jun 3, 2019

@bors bors merged commit de2bf3a into rust-lang:master Jun 3, 2019

2 checks passed

Travis CI - Pull Request Build Passed
Details
homu Test successful
Details

@varkor varkor deleted the varkor:must_use-tuple-expr branch Jun 3, 2019

@scottmcm

This comment has been minimized.

Copy link
Member

commented Jun 3, 2019

This seems bad in terms of justifying adding the lint

I don't think crater should necessarily be expected to find lint warnings, @Centril, because it's code that theoretically works. It can't tell use whether someone wrote the code wrong, had to debug to find it, fixed it, and then uploaded to crates.io once it worked.

bors added a commit that referenced this pull request Jul 22, 2019
Auto merge of #62262 - varkor:must_use-adt-components-ii, r=<try>
Extend `#[must_use]` to nested structures

Extends the `#[must_use]` lint to apply when `#[must_use]` types are nested within `struct`s (or one-variant `enum`s), making the lint much more generally useful. This is in line with #61100 extending the lint to tuples.

Fixes #39524.

cc @rust-lang/lang and @rust-lang/compiler for discussion in case this is a controversial change. In particular, we might want to consider allowing annotations on fields containing `#[must_use]` types in user-defined types (e.g. `#[allow(unused_must_use)]`) to opt out of this behaviour, if there are cases where we this this is likely to have frequent false positives.

(This is based on top of #62235.)
bors added a commit that referenced this pull request Jul 22, 2019
Auto merge of #62262 - varkor:must_use-adt-components-ii, r=<try>
Extend `#[must_use]` to nested structures

Extends the `#[must_use]` lint to apply when `#[must_use]` types are nested within `struct`s (or one-variant `enum`s), making the lint much more generally useful. This is in line with #61100 extending the lint to tuples.

Fixes #39524.

cc @rust-lang/lang and @rust-lang/compiler for discussion in case this is a controversial change. In particular, we might want to consider allowing annotations on fields containing `#[must_use]` types in user-defined types (e.g. `#[allow(unused_must_use)]`) to opt out of this behaviour, if there are cases where we this this is likely to have frequent false positives.

(This is based on top of #62235.)
bors added a commit that referenced this pull request Jul 24, 2019
Auto merge of #62262 - varkor:must_use-adt-components-ii, r=<try>
Extend `#[must_use]` to nested structures

Extends the `#[must_use]` lint to apply when `#[must_use]` types are nested within `struct`s (or one-variant `enum`s), making the lint much more generally useful. This is in line with #61100 extending the lint to tuples.

Fixes #39524.

cc @rust-lang/lang and @rust-lang/compiler for discussion in case this is a controversial change. In particular, we might want to consider allowing annotations on fields containing `#[must_use]` types in user-defined types (e.g. `#[allow(unused_must_use)]`) to opt out of this behaviour, if there are cases where we this this is likely to have frequent false positives.

(This is based on top of #62235.)
bors added a commit that referenced this pull request Jul 30, 2019
Auto merge of #62262 - varkor:must_use-adt-components-ii, r=<try>
Extend `#[must_use]` to nested structures

Extends the `#[must_use]` lint to apply when `#[must_use]` types are nested within `struct`s (or one-variant `enum`s), making the lint much more generally useful. This is in line with #61100 extending the lint to tuples.

Fixes #39524.

cc @rust-lang/lang and @rust-lang/compiler for discussion in case this is a controversial change. In particular, we might want to consider allowing annotations on fields containing `#[must_use]` types in user-defined types (e.g. `#[allow(unused_must_use)]`) to opt out of this behaviour, if there are cases where we this this is likely to have frequent false positives.

(This is based on top of #62235.)
netbsd-srcmastr pushed a commit to NetBSD/pkgsrc that referenced this pull request Aug 29, 2019
he
Update rust to version 1.37.0
Pkgsrc changes:
 * Add a patch to llvm to deal with const dli_saddr.
 * Adapt two other patches.
 * Cross-build currently fails, so i386, powerpc and sparc64 bootstrap
   kits for 1.37.0 are built natively.  Missing aarch64 hardware, so that's
   not available yet.
 * Bump bootstrap requirements to 1.36.0 except for armv7-unknown-netbsd-eabihf
   which I've not managed to cross-build.

Upstream changes:

Version 1.37.0 (2019-08-15)
==========================

Language
--------
- `#[must_use]` will now warn if the type is contained in a [tuple][61100],
  [`Box`][62228], or an [array][62235] and unused.
- [You can now use the `cfg` and `cfg_attr` attributes on
  generic parameters.][61547]
- [You can now use enum variants through type alias.][61682] e.g. You can
  write the following:
  ```rust
  type MyOption = Option<u8>;

  fn increment_or_zero(x: MyOption) -> u8 {
      match x {
          MyOption::Some(y) => y + 1,
          MyOption::None => 0,
      }
  }
  ```
- [You can now use `_` as an identifier for consts.][61347] e.g. You can write
  `const _: u32 = 5;`.
- [You can now use `#[repr(align(X)]` on enums.][61229]
- [The  `?`/_"Kleene"_ macro operator is now available in the
  2015 edition.][60932]

Compiler
--------
- [You can now enable Profile-Guided Optimization with the `-C profile-generate`
  and `-C profile-use` flags.][61268] For more information on how to use profile
  guided optimization, please refer to the [rustc book][rustc-book-pgo].
- [The `rust-lldb` wrapper script should now work again.][61827]

Libraries
---------
- [`mem::MaybeUninit<T>` is now ABI-compatible with `T`.][61802]

Stabilized APIs
---------------
- [`BufReader::buffer`]
- [`BufWriter::buffer`]
- [`Cell::from_mut`]
- [`Cell<[T]>::as_slice_of_cells`][`Cell<slice>::as_slice_of_cells`]
- [`DoubleEndedIterator::nth_back`]
- [`Option::xor`]
- [`Wrapping::reverse_bits`]
- [`i128::reverse_bits`]
- [`i16::reverse_bits`]
- [`i32::reverse_bits`]
- [`i64::reverse_bits`]
- [`i8::reverse_bits`]
- [`isize::reverse_bits`]
- [`slice::copy_within`]
- [`u128::reverse_bits`]
- [`u16::reverse_bits`]
- [`u32::reverse_bits`]
- [`u64::reverse_bits`]
- [`u8::reverse_bits`]
- [`usize::reverse_bits`]

Cargo
-----
- [`Cargo.lock` files are now included by default when publishing executable crates
  with executables.][cargo/7026]
- [You can now specify `default-run="foo"` in `[package]` to specify the
  default executable to use for `cargo run`.][cargo/7056]

Misc
----

Compatibility Notes
-------------------
- [Using `...` for inclusive range patterns will now warn by default.][61342]
  Please transition your code to using the `..=` syntax for inclusive
  ranges instead.
- [Using a trait object without the `dyn` will now warn by default.][61203]
  Please transition your code to use `dyn Trait` for trait objects instead.

[62228]: rust-lang/rust#62228
[62235]: rust-lang/rust#62235
[61802]: rust-lang/rust#61802
[61827]: rust-lang/rust#61827
[61547]: rust-lang/rust#61547
[61682]: rust-lang/rust#61682
[61268]: rust-lang/rust#61268
[61342]: rust-lang/rust#61342
[61347]: rust-lang/rust#61347
[61100]: rust-lang/rust#61100
[61203]: rust-lang/rust#61203
[61229]: rust-lang/rust#61229
[60932]: rust-lang/rust#60932
[cargo/7026]: rust-lang/cargo#7026
[cargo/7056]: rust-lang/cargo#7056
[`BufReader::buffer`]: https://doc.rust-lang.org/std/io/struct.BufReader.html#method.buffer
[`BufWriter::buffer`]: https://doc.rust-lang.org/std/io/struct.BufWriter.html#method.buffer
[`Cell::from_mut`]: https://doc.rust-lang.org/std/cell/struct.Cell.html#method.from_mut
[`Cell<slice>::as_slice_of_cells`]: https://doc.rust-lang.org/std/cell/struct.Cell.html#method.as_slice_of_cells
[`DoubleEndedIterator::nth_back`]: https://doc.rust-lang.org/std/iter/trait.DoubleEndedIterator.html#method.nth_back
[`Option::xor`]: https://doc.rust-lang.org/std/option/enum.Option.html#method.xor
[`RefCell::try_borrow_unguarded`]: https://doc.rust-lang.org/std/cell/struct.RefCell.html#method.try_borrow_unguarded
[`Wrapping::reverse_bits`]: https://doc.rust-lang.org/std/num/struct.Wrapping.html#method.reverse_bits
[`i128::reverse_bits`]: https://doc.rust-lang.org/std/primitive.i128.html#method.reverse_bits
[`i16::reverse_bits`]: https://doc.rust-lang.org/std/primitive.i16.html#method.reverse_bits
[`i32::reverse_bits`]: https://doc.rust-lang.org/std/primitive.i32.html#method.reverse_bits
[`i64::reverse_bits`]: https://doc.rust-lang.org/std/primitive.i64.html#method.reverse_bits
[`i8::reverse_bits`]: https://doc.rust-lang.org/std/primitive.i8.html#method.reverse_bits
[`isize::reverse_bits`]: https://doc.rust-lang.org/std/primitive.isize.html#method.reverse_bits
[`slice::copy_within`]: https://doc.rust-lang.org/std/primitive.slice.html#method.copy_within
[`u128::reverse_bits`]: https://doc.rust-lang.org/std/primitive.u128.html#method.reverse_bits
[`u16::reverse_bits`]: https://doc.rust-lang.org/std/primitive.u16.html#method.reverse_bits
[`u32::reverse_bits`]: https://doc.rust-lang.org/std/primitive.u32.html#method.reverse_bits
[`u64::reverse_bits`]: https://doc.rust-lang.org/std/primitive.u64.html#method.reverse_bits
[`u8::reverse_bits`]: https://doc.rust-lang.org/std/primitive.u8.html#method.reverse_bits
[`usize::reverse_bits`]: https://doc.rust-lang.org/std/primitive.usize.html#method.reverse_bits
[rustc-book-pgo]: https://doc.rust-lang.org/rustc/profile-guided-optimization.html
bors added a commit that referenced this pull request Sep 9, 2019
Auto merge of #62262 - varkor:must_use-adt-components-ii, r=<try>
Extend `#[must_use]` to nested structures

Extends the `#[must_use]` lint to apply when `#[must_use]` types are nested within `struct`s (or one-variant `enum`s), making the lint much more generally useful. This is in line with #61100 extending the lint to tuples.

Fixes #39524.

cc @rust-lang/lang and @rust-lang/compiler for discussion in case this is a controversial change. In particular, we might want to consider allowing annotations on fields containing `#[must_use]` types in user-defined types (e.g. `#[allow(unused_must_use)]`) to opt out of this behaviour, if there are cases where we this this is likely to have frequent false positives.

(This is based on top of #62235.)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
8 participants
You can’t perform that action at this time.