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

debuginfo: Improve commandline option handling for debuginfo #12816

Closed

Conversation

michaelwoerister
Copy link
Member

Fixes #12811 as described in the issue.

}
}
None => NoDebugInfo
} else {
NoDebugInfo
Copy link
Member

Choose a reason for hiding this comment

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

Instead of using unwrap(), this may be better expressed with a match where the None case returns NoDebugInfo. Could you also put the {} in instead was {} in backticks? (just to be clear what the user input was)

Additionally, having ~str as a pattern will likely be going away, so it may be easier to go ahead and remove it for now (matching on Some("0") instead for example)

Copy link
Member Author

Choose a reason for hiding this comment

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

Sure, I'll do that. The code in question was pretty much literally copied from --opt-level handling directly above it. I'll adapt this too...

@alexcrichton
Copy link
Member

Ah one last thing, for historical purposes, could you expand the commit message as well? (just a sentence or two)

match matches.opt_str("debuginfo").unwrap() {
~"0" => NoDebugInfo,
~"1" => LimitedDebugInfo,
~"2" => FullDebugInfo,
Copy link
Contributor

Choose a reason for hiding this comment

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

This will rot #12756. You might want to match against a slice if we want to be inline with it.
Edit: disregard, seems like it will need to be rebased anyway.

@michaelwoerister
Copy link
Member Author

Like this?

Also, matching on Some("...") was not possible due to lifetime issues (although I thought that it should be easy). I'd be interested in an alternative formulation to mine. Maybe Option needs a map that borrows instead of moving?

@jdm
Copy link
Contributor

jdm commented Mar 10, 2014

I think that's usually formulated as opt.as_ref().map.

@michaelwoerister
Copy link
Member Author

I think that's usually formulated as opt.as_ref().map.

D'oh! Well, I'll better just go to bed now. Seems like a good ide :)

…ust-lang#12811)

The `-g` flag does not take an argument anymore while the argument to `--debuginfo` becomes mandatory. This change makes it possible again to run the compiler like this:

`rustc -g ./file.rs`

This did not work before because `./file.rs` was misinterpreted as the argument to `-g`. In order to get limited debuginfo, one now has to use `--debuginfo=1`.
@michaelwoerister
Copy link
Member Author

This just received an update (rebased, extended commit message, code cleanup)

bors added a commit that referenced this pull request Mar 12, 2014
@bors bors closed this Mar 12, 2014
flip1995 pushed a commit to flip1995/rust that referenced this pull request Jul 11, 2024
Don't lint `assertions_on_constants` on any const assertions

close rust-lang#12816
close rust-lang#12847
cc rust-lang#12817

----

changelog: Fix false positives in consts for `assertions_on_constants` and `unnecessary_operation`.
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.

rustc -g file.rs does not work
5 participants