Skip to content

Fix some problems #3535

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

Merged
merged 6 commits into from
Dec 12, 2018
Merged

Fix some problems #3535

merged 6 commits into from
Dec 12, 2018

Conversation

sinkuu
Copy link
Contributor

@sinkuu sinkuu commented Dec 12, 2018

Fixes #2892, #3199, #2841, #3476

@phansch phansch added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Dec 12, 2018
@phansch
Copy link
Member

phansch commented Dec 12, 2018

Is there any reason for 328e105? Otherwise r=me 🎇

@bors
Copy link
Contributor

bors commented Dec 12, 2018

☔ The latest upstream changes (presumably #3529) made this pull request unmergeable. Please resolve the merge conflicts.

@phansch phansch added S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status) and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties labels Dec 12, 2018
@sinkuu
Copy link
Contributor Author

sinkuu commented Dec 12, 2018

These are not equivalent:

let _ = if self.opt.is_none() {
    /* no `return` here */ None
} else {
    self.opt
};
let _ = self.opt?;

I'll add this as a comment along with conflict resolution.

if opt.is_none() { None } else { opt } can be changed to opt, but that may be out of scope of this lint ("question_mark").

@oli-obk
Copy link
Contributor

oli-obk commented Dec 12, 2018

let x = self.opt.is_none() {
    return None;
} else {
    self.op
};

is also not

let _ = self.opt?;

We can always suggest let _ = Some(self.opt?); and have the user take it from there.

@phansch
Copy link
Member

phansch commented Dec 12, 2018

@bors r+ thanks!

@bors
Copy link
Contributor

bors commented Dec 12, 2018

📌 Commit eba44e1 has been approved by phansch

@phansch phansch removed the S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status) label Dec 12, 2018
@bors
Copy link
Contributor

bors commented Dec 12, 2018

⌛ Testing commit eba44e1 with merge 379c934...

bors added a commit that referenced this pull request Dec 12, 2018
@bors
Copy link
Contributor

bors commented Dec 12, 2018

☀️ Test successful - status-appveyor, status-travis
Approved by: phansch
Pushing 379c934 to master...

@bors bors merged commit eba44e1 into rust-lang:master Dec 12, 2018
@sinkuu sinkuu deleted the fixes branch December 12, 2018 23:33
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.

4 participants