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.

Sign up for GitHub

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

Jump to bottom

Remove ObsoleteInPlace #60803

Merged
merged 2 commits into from
May 24, 2019
Merged

Remove ObsoleteInPlace #60803

merged 2 commits into from
May 24, 2019

Conversation

varkor
Copy link
Member

@varkor varkor commented May 13, 2019

The in place syntax has been deprecated for over a year. As it is, this is accumulated cruft: the error messages are unlikely to be helpful any more and it conflicts with some useful syntax (e.g. const generics in some instances).

It may be that removing Token::LArrow is backwards-incompatible. We should do a crater run to check.

cc @eddyb

@varkor

This comment has been minimized.

Sign in to view
@bors

This comment has been minimized.

Sign in to view
@rust-highfive

This comment has been minimized.

Sign in to view
@petrochenkov petrochenkov self-assigned this May 13, 2019
Centril
Centril reviewed May 13, 2019
View reviewed changes
src/test/ui/placement-syntax.rs Outdated
fn main() {
let x = -5;
if x<-1 {
//~^ ERROR emplacement syntax is obsolete
if x<-1 { // ok: parses as a comparison
Copy link
Contributor

Choose a reason for hiding this comment

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

So I think it's good that we remove it from the AST and such, but keeping options open re. <- and not introducing new stable behavior would be good so let's reject this in the parser?

@petrochenkov
Copy link
Contributor

Removing ObsoleteInPlace is a great idea since the deprecation was long enough and it's very unlikely to be used under cfg(FALSE) anymore.
I'm skeptical a removing the <- token though, it's there for future-proofing at least (it existed with this purpose even before emplacement was actually implemented).

@petrochenkov
Copy link
Contributor

Let's see what crater finds anyway.

@petrochenkov
Copy link
Contributor

r? @petrochenkov

@petrochenkov petrochenkov added the S-waiting-on-crater Status: Waiting on a crater run to be completed. label May 13, 2019
@varkor

This comment has been minimized.

Sign in to view
@craterbot

This comment has been minimized.

Sign in to view
@craterbot

This comment has been minimized.

Sign in to view
@pietroalbini

This comment has been minimized.

Sign in to view
@craterbot

This comment has been minimized.

Sign in to view
@craterbot craterbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-crater Status: Waiting on a crater run to be completed. labels May 14, 2019
@petrochenkov

This comment has been minimized.

Sign in to view
@petrochenkov petrochenkov added S-waiting-on-crater Status: Waiting on a crater run to be completed. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 15, 2019
@bors

This comment has been minimized.

Sign in to view
@yodaldevoid

This comment has been minimized.

Sign in to view
@pietroalbini

This comment has been minimized.

Sign in to view
@bors

This comment has been minimized.

Sign in to view
@pietroalbini pietroalbini reopened this May 18, 2019
@varkor varkor changed the title Remove ObsoleteInPlace and LArrow Remove ObsoleteInPlace May 22, 2019
@rust-highfive

This comment has been minimized.

Sign in to view
@bors

This comment has been minimized.

Sign in to view
@varkor varkor force-pushed the remove-in-place-syntax branch 2 times, most recently from 095877a to 36f6542 Compare May 24, 2019 00:27
@varkor
Copy link
Member Author

varkor commented May 24, 2019

Removed the <- token removal.

@petrochenkov
Copy link
Contributor

@bors r+

@bors
Copy link
Contributor

bors commented May 24, 2019

📌 Commit 36f6542 has been approved by petrochenkov

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels May 24, 2019
@bors
Copy link
Contributor

bors commented May 24, 2019

⌛ Testing commit 36f6542 with merge d96c01e...

bors added a commit that referenced this pull request May 24, 2019
@bors
Auto merge of #60803 - varkor:remove-in-place-syntax, r=petrochenkov
d96c01e 
Remove `ObsoleteInPlace`

The in place syntax has been deprecated for over a year. As it is, this is accumulated cruft: the error messages are unlikely to be helpful any more and it conflicts with some useful syntax (e.g. const generics in some instances).

It may be that removing `Token::LArrow` is backwards-incompatible. We should do a crater run to check.

cc @eddyb
@bors
Copy link
Contributor

bors commented May 24, 2019

☀️ Test successful - checks-travis, status-appveyor
Approved by: petrochenkov
Pushing d96c01e to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label May 24, 2019
@bors bors merged commit 36f6542 into rust-lang:master May 24, 2019
@bors bors mentioned this pull request May 24, 2019
Merged
matthiaskrgr added a commit to matthiaskrgr/rust-clippy that referenced this pull request May 24, 2019
@matthiaskrgr
rustup rust-lang/rust#60803
c67d2b1
bors added a commit to rust-lang/rust-clippy that referenced this pull request May 24, 2019
@bors
Auto merge of #4135 - matthiaskrgr:rustup_obsol, r=phansch
d519ed4 
rustup rust-lang/rust#60803

changelog: none
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request May 25, 2019
@matthiaskrgr
submodules: update clippy from a8eeb7c to 577ee79
eb98952 
Changes:
````
Rustup to rust-lang#61026
rustup rust-lang#60803
Rustup to rust-lang#59545
Rustup to rust-lang#60965
clippy: bump rustc_tools util version to 0.2 rustc_tools_util: fix typo in docs (readme)
rustc_tool_utils: bump version to 0.2.0
update if_chain to 1.0.0
tests: update needless_bool test stderr
cargo fmt
Rustup to rust-lang#60740
Lifetimes UI test cleanup
````
@Centril Centril mentioned this pull request Jul 1, 2019
Open
@Mark-Simulacrum Mark-Simulacrum mentioned this pull request Jul 12, 2019
Closed
@ehuss ehuss mentioned this pull request Jul 12, 2019
Closed
estebank
estebank reviewed Jul 12, 2019
View reviewed changes
src/test/ui/placement-syntax.stderr
help: if you meant to write a comparison against a negative value, add a space in between `<` and `-`
|
LL | if x< -1 {
| ^^^
Copy link
Contributor

Choose a reason for hiding this comment

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

It is a shame that we lost this suggestion...

Copy link
Member Author

Choose a reason for hiding this comment

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

Added #62632 to track this.

@varkor varkor mentioned this pull request Jul 12, 2019
Closed
@varkor varkor deleted the remove-in-place-syntax branch July 12, 2019 20:03
@tmandry tmandry mentioned this pull request Dec 12, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Centril Centril Centril left review comments

@estebank estebank estebank left review comments

Assignees

@petrochenkov petrochenkov

Labels
merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
10 participants
@varkor @bors @rust-highfive @petrochenkov @craterbot @pietroalbini @yodaldevoid @Centril @estebank @michaelwoerister