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

Ignore non-const ctor expressions in or_fn_call #4018

Merged
merged 2 commits into from Apr 23, 2019

Conversation

Projects
None yet
3 participants
@Manishearth
Copy link
Member

commented Apr 22, 2019

Fixes #1338

Should have been fixed by #919, however that focuses on const ctor expressions only, and .or(Some(local)) isn't const.

This also automatically ignores things like .or(Some(local.clone()) which we don't actually want to do; I need to figure out what to do here.

changelog: Fixed false positive in [or_fn_call] pertaining to enum variant constructors

r? @oli-obk @phansch

@Manishearth

This comment has been minimized.

Copy link
Member Author

commented Apr 22, 2019

The fully general solution is to use a visitor and make sure that the .or(foo(bar(baz))) is composed purely of ctors, locals, or consts.

Mind me filing a separate bug for this case so we can land this?

@oli-obk

This comment has been minimized.

Copy link
Collaborator

commented Apr 23, 2019

@bors r+

@bors

This comment has been minimized.

Copy link
Contributor

commented Apr 23, 2019

📌 Commit 9a92e26 has been approved by oli-obk

@bors

This comment has been minimized.

Copy link
Contributor

commented Apr 23, 2019

⌛️ Testing commit 9a92e26 with merge 23bc744...

bors added a commit that referenced this pull request Apr 23, 2019

Auto merge of #4018 - rust-lang:or_fn_call_variants, r=oli-obk
Ignore non-const ctor expressions in or_fn_call

Fixes #1338

Should have been fixed by #919, however that focuses on const ctor expressions only, and `.or(Some(local))` isn't const.

This also automatically ignores things like `.or(Some(local.clone())` which we don't actually want to do; I need to figure out what to do here.

changelog: Fixed false positive in [`or_fn_call`] pertaining to enum variant constructors

r? @oli-obk @phansch
@bors

This comment has been minimized.

Copy link
Contributor

commented Apr 23, 2019

💔 Test failed - checks-travis

@oli-obk

This comment has been minimized.

Copy link
Collaborator

commented Apr 23, 2019

r=me with rustfmt applied

@Manishearth Manishearth force-pushed the or_fn_call_variants branch from 9a92e26 to b03cf3f Apr 23, 2019

@Manishearth

This comment has been minimized.

Copy link
Member Author

commented Apr 23, 2019

@bors r=oli-obk

ah, no wonder it wasn't getting formatted earlier, i'd forgotten --all

@bors

This comment has been minimized.

Copy link
Contributor

commented Apr 23, 2019

📌 Commit b03cf3f has been approved by oli-obk

bors added a commit that referenced this pull request Apr 23, 2019

Auto merge of #4018 - rust-lang:or_fn_call_variants, r=oli-obk
Ignore non-const ctor expressions in or_fn_call

Fixes #1338

Should have been fixed by #919, however that focuses on const ctor expressions only, and `.or(Some(local))` isn't const.

This also automatically ignores things like `.or(Some(local.clone())` which we don't actually want to do; I need to figure out what to do here.

changelog: Fixed false positive in [`or_fn_call`] pertaining to enum variant constructors

r? @oli-obk @phansch
@bors

This comment has been minimized.

Copy link
Contributor

commented Apr 23, 2019

⌛️ Testing commit b03cf3f with merge eccb85f...

@bors

This comment has been minimized.

Copy link
Contributor

commented Apr 23, 2019

💔 Test failed - status-appveyor

@Manishearth

This comment has been minimized.

Copy link
Member Author

commented Apr 23, 2019

needs rustup

@Manishearth

This comment has been minimized.

Copy link
Member Author

commented Apr 23, 2019

@bors retry

@bors

This comment has been minimized.

Copy link
Contributor

commented Apr 23, 2019

⌛️ Testing commit b03cf3f with merge 9897442...

bors added a commit that referenced this pull request Apr 23, 2019

Auto merge of #4018 - rust-lang:or_fn_call_variants, r=oli-obk
Ignore non-const ctor expressions in or_fn_call

Fixes #1338

Should have been fixed by #919, however that focuses on const ctor expressions only, and `.or(Some(local))` isn't const.

This also automatically ignores things like `.or(Some(local.clone())` which we don't actually want to do; I need to figure out what to do here.

changelog: Fixed false positive in [`or_fn_call`] pertaining to enum variant constructors

r? @oli-obk @phansch
@Manishearth

This comment has been minimized.

Copy link
Member Author

commented Apr 23, 2019

The rust queue has a rollup waiting so might as well squeeze this in before I land the submodule update

@bors

This comment has been minimized.

Copy link
Contributor

commented Apr 23, 2019

☀️ Test successful - checks-travis, status-appveyor
Approved by: oli-obk
Pushing 9897442 to master...

@bors bors merged commit b03cf3f into master Apr 23, 2019

2 of 5 checks passed

Travis CI - Branch Build Failed
Details
continuous-integration/appveyor/branch AppVeyor build failed
Details
continuous-integration/appveyor/pr AppVeyor build failed
Details
Travis CI - Pull Request Build Passed
Details
homu Test successful
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.