Skip to content

Fix tests most unused pattern variable warnings #3286

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

Closed
wants to merge 6 commits into from

Conversation

erickt
Copy link
Contributor

@erickt erickt commented Aug 26, 2012

I almost fixed all the unused variable warnings, and found a couple cases where we weren't properly using an enum because it wasn't imported. There were a couple I didn't know how to handle though:

/Users/etryzelaar/Projects/rust/rust/src/rustc/middle/ty.rs:1038:6: 1038:27 warning: unused variable: `sty`
/Users/etryzelaar/Projects/rust/rust/src/rustc/middle/ty.rs:1038       ref sty @ ty_fn(f) => {
                                                                       ^~~~~~~~~~~~~~~~~~~~~
/Users/etryzelaar/Projects/rust/rust/src/rustc/middle/ty.rs:2232:6: 2232:22 warning: unused variable: `re_bot`
/Users/etryzelaar/Projects/rust/rust/src/rustc/middle/ty.rs:2232       re_bot        => 4u
                                                                       ^~~~~~~~~~~~~~~~
/Users/etryzelaar/Projects/rust/rust/src/rustc/middle/typeck/check.rs:895:14: 895:43 warning: unused variable: `sty`
/Users/etryzelaar/Projects/rust/rust/src/rustc/middle/typeck/check.rs:895               sty @ ty::ty_fn(ref fn_ty) => {
                                                                                        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~

I'm not quite sure what the sty @ ty::ty_fn(...) means, and re_bot seems like it should be a variant, but it doesn't exist.

@brson
Copy link
Contributor

brson commented Aug 27, 2012

@nikomatsakis perhaps you can review this. Are we confident that none of these unused patterns indicate bugs?

@erickt
Copy link
Contributor Author

erickt commented Aug 27, 2012

By the way, I just rebased this onto HEAD to merge in the Some/None changes.

@nikomatsakis
Copy link
Contributor

@brson I'll take a look.

@@ -378,7 +378,7 @@ impl printer {
if !self.scan_stack_empty {
let x = self.scan_top();
match copy self.token[x] {
BEGIN(b) => {
BEGIN(_) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

@graydon this was one line that I was unsure about. It seemed like it could be a bug...? But prob not

@nikomatsakis
Copy link
Contributor

I think it's pretty reasonable. There's only so many of those diffs you can look at until your eyes glaze over, but I didn't see anything that looked suspicious.

@brson
Copy link
Contributor

brson commented Aug 27, 2012

This is merged.

@brson brson closed this Aug 27, 2012
RalfJung pushed a commit to RalfJung/rust that referenced this pull request Feb 17, 2024
jaisnan pushed a commit to jaisnan/rust-dev that referenced this pull request Jul 29, 2024
Dependency upgrade resulting from `cargo update`.

Co-authored-by: tautschnig <[email protected]>
Co-authored-by: Zyad Hassan <[email protected]>
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.

3 participants