-
Notifications
You must be signed in to change notification settings - Fork 12.9k
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
[Do not merge] Optional turbofish breakage test #53578
Conversation
@bors try |
[Do not merge] Optional turbofish breakage test As an academic exercise, to satisfy ourselves that we completely understand the lay of the land, we'd like to see how often ambiguous cases arise in real-world Rust code. (Following on from #53511.) @rust-lang/infra: is it possible to get a check-only crater run for this change? r? @ghost
☀️ Test successful - status-travis |
@rust-lang/infra could we get a crater run? |
@craterbot run start=master#d0d81b7fc1421859ba0218e8a437af29ae3b0967 end=try#a5b9f2cc0cdaca8edf23a89c649279de2c91cd3b mode=check-only |
👌 Experiment ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more |
🚧 Experiment ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more |
🎉 Experiment
|
🤔 526 regressed. The number is higher than I expected. Almost all of the regressions are caused by the same line in
Looks the parser is trying to parse something similar to Other breakages are similar. No breakage is caused by actual ambiguity. microbench
|
@kennytm: thanks for taking a look at the regressions! It's taken a while for me to look at this. It looks like it might be possible to make the error reporting in this case lazier and backtrack instead.
Possibly; I can't see why this change would affect it. |
01cb485
to
ca2849a
Compare
@bors try |
[Do not merge] Optional turbofish breakage test As an academic exercise, to satisfy ourselves that we completely understand the lay of the land, we'd like to see how often ambiguous cases arise in real-world Rust code. (Following on from #53511.) @rust-lang/infra: is it possible to get a check-only crater run for this change? r? @ghost
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
ca2849a
to
307ea60
Compare
@bors try |
⌛ Trying commit 307ea60 with merge 102de0ab47505b4e9d03054e600bcac5ad4939b7... |
@bors try |
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
Hopefully that's the only thing that fails the test 😄 |
(Huh, the previous try didn't work.) @bors try |
⌛ Trying commit c7b5c52 with merge 87b8e7411c4f1267d0cac0d63fa9c21c943e2ea0... |
☀️ Test successful - status-travis |
@craterbot run start=master#90d36fb5905bbe5004f5b465ea14b53d10dae260 end=try#87b8e7411c4f1267d0cac0d63fa9c21c943e2ea0 mode=check-only |
🚨 Error: an experiment named 🆘 If you have any trouble with Crater please ping |
@craterbot run start=master#90d36fb5905bbe5004f5b465ea14b53d10dae260 end=try#87b8e7411c4f1267d0cac0d63fa9c21c943e2ea0 mode=check-only name=pr-53578-1 |
👌 Experiment ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more |
🚧 Experiment ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more |
🎉 Experiment
|
The one regression is spurious. It seems switching the syntax to |
Agreeing with @petrochenkov and @SimonSapin (here and in the RFCS) that making this change for 2015 edition could be unwise, but I would love to have this PR merged with customized suggestion when this would work to actually suggest using the turbofish. The same reasoning behind changing the semantics applies to making the diagnostic error more accurate. What do you think, @varkor? Would you be ok with cleaning up this PR as a diagnostic only change while the RFCS process and conversation marches on? |
To clarify, I think making this change for any edition is unwise, but the theoretical breakage is probably the last of the reasons for that. |
Triage; @varkor Hello, what is the current status of this PR? |
For now, let's just close this; I'll come back to it after the 2018 Edition is out. |
As an academic exercise, to satisfy ourselves that we completely understand the lay of the land, we'd like to see how often ambiguous cases arise in real-world Rust code. (Following on from #53511.)
@rust-lang/infra: is it possible to get a check-only crater run for this change?
r? @ghost