syntax: Remove warning for unnecessary path disambiguators by petrochenkov · Pull Request #57565 · rust-lang/rust

petrochenkov · 2019-01-13T02:09:06Z

rustfmt is now stable and it removes unnecessary turbofishes, so removing the warning as discussed in #43540 (where it was introduced).
One hardcoded warning less.

Closes #58055

r? @nikomatsakis

nikomatsakis · 2019-01-15T16:31:12Z

@rfcbot fcp merge

There used to be a hard-coded warning for unnecessary :: in a path (e.g., Vec::<u32> as a type). As @petrochenkov notes, this is no longer needed, since rustfmt will silently normalize this. In general, I favor using rustfmt over warnings to try and push minor stylistic nits -- far less annoying. But in this particular case, it would also be good, because this warning appears so early we can't easily use the standard lint system, so it cannot be "allowed" or "denied" etc. Therefore, I am in favor of merging this PR and removing the warning.

rfcbot · 2019-01-15T16:31:13Z

Team member @nikomatsakis has proposed to merge this. The next step is review by the rest of the tagged team members:

No concerns currently listed.

Once a majority of reviewers approve (and none object), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

Centril · 2019-01-15T16:37:54Z

I feel that this style is distinctly unidiomatic and would therefore fit well into nonstandard_style; however, if that is exceedingly difficult to do well in the compiler (e.g. it would slow things down or incur very ugly hacks) I could be swayed to not lint anymore. Feels like a good topic to discuss on Thursday.

nikomatsakis · 2019-01-17T20:09:44Z

@petrochenkov question -- can we set a flag in the resulting AST to indicate that an unnecessary :: was present, in which case we could make a "true lint", right?

nikomatsakis · 2019-01-17T20:10:55Z

I'm still happy for rustfmt to silently normalize, but @cramertj feels like there is value in having a "teachable moment" here -- but it'd be nice if it were rustfix'able or something like that. I can certainly see that.

petrochenkov · 2019-01-17T22:22:04Z

can we set a flag in the resulting AST to indicate that an unnecessary :: was present

Ugh, we can but that would at least increase the size of every path segment in AST.

The hardcoded warning can be converted into a lint without extending AST, but it will be attached to the crate root and only could be allowed/denyed at crate granularity.

nikomatsakis · 2019-01-18T19:40:51Z

The hardcoded warning can be converted into a lint without extending AST, but it will be attached to the crate root and only could be allowed/denyed at crate granularity.

Sure, but that seems pretty surprising. I guess it could be considered a long-standing bug.

Presumably we could do something where we record the span, too, and try to reproduce the node-id from that.

nikomatsakis · 2019-01-18T19:41:10Z

To be clear, I don't know that it's worth the effort.

nikomatsakis · 2019-01-18T19:41:29Z

Particularly not if we adopt the "remove turbofish" RFC, in which case... nobody would likely think to add :: in the first place.

rfcbot · 2019-01-26T11:51:51Z

🔔 This is now entering its final comment period, as per the review above. 🔔

rfcbot · 2019-02-05T11:52:04Z

The final comment period, with a disposition to merge, as per the review above, is now complete.

Dylan-DPC-zz · 2019-03-04T09:46:52Z

ping from triage @petrochenkov you have a conflict to resolve

petrochenkov · 2019-03-04T18:08:49Z

Rebased.

Dylan-DPC-zz · 2019-03-18T22:41:20Z

ping from triage @petrochenkov more conflicts :D (maybe ask for a r+ before more conflicts arise)

petrochenkov · 2019-03-19T06:06:02Z

@Dylan-DPC
The S-waiting-on-review label is already asking for r+.
(Also, conflicts don't usually prevent reviewing, that's why rfcbot doesn't reset the label to S-waiting-on-author when they happen, so the triage team probably shouldn't do it either.)

Dylan-DPC-zz · 2019-03-19T10:38:05Z

@petrochenkov true though, but i have seen some reviewers wait for conflicts to be resolved before reviewing (especially if the conflict relates to the change being made).

Yeah i know the label is for that, but sometimes reviewers oversee that and won't check a PR until they are pinged or there is a discussion somewhere.

Centril · 2019-03-26T08:15:01Z

Reassigning to myself to lessen Niko's review load; r? @Centril

@bors r+

bors · 2019-03-26T08:15:02Z

📌 Commit 4b38294 has been approved by Centril

@nikomatsakis

syntax: Remove warning for unnecessary path disambiguators `rustfmt` is now stable and it removes unnecessary turbofishes, so removing the warning as discussed in rust-lang#43540 (where it was introduced). One hardcoded warning less. Closes rust-lang#58055 r? @nikomatsakis

@nikomatsakis

syntax: Remove warning for unnecessary path disambiguators `rustfmt` is now stable and it removes unnecessary turbofishes, so removing the warning as discussed in rust-lang#43540 (where it was introduced). One hardcoded warning less. Closes rust-lang#58055 r? @nikomatsakis

@nikomatsakis

syntax: Remove warning for unnecessary path disambiguators `rustfmt` is now stable and it removes unnecessary turbofishes, so removing the warning as discussed in rust-lang#43540 (where it was introduced). One hardcoded warning less. Closes rust-lang#58055 r? @nikomatsakis

@nikomatsakis

syntax: Remove warning for unnecessary path disambiguators `rustfmt` is now stable and it removes unnecessary turbofishes, so removing the warning as discussed in rust-lang#43540 (where it was introduced). One hardcoded warning less. Closes rust-lang#58055 r? @nikomatsakis

@ghost

Rollup of 18 pull requests Successful merges: - #57293 (Make some lints incremental) - #57565 (syntax: Remove warning for unnecessary path disambiguators) - #58253 (librustc_driver => 2018) - #58837 (librustc_interface => 2018) - #59268 (Add suggestion to use `&*var` when `&str: From<String>` is expected) - #59283 (Make ASCII case conversions more than 4× faster) - #59284 (adjust MaybeUninit API to discussions) - #59372 (add rustfix-able suggestions to trim_{left,right} deprecations) - #59390 (Make `ptr::eq` documentation mention fat-pointer behavior) - #59393 (Refactor tuple comparison tests) - #59420 ([CI] record docker image info for reuse) - #59421 (Reject integer suffix when tuple indexing) - #59430 (Renames `EvalContext` to `InterpretCx`) - #59439 (Generalize diagnostic for `x = y` where `bool` is the expected type) - #59449 (fix: Make incremental artifact deletion more robust) - #59451 (Add `Default` to `std::alloc::System`) - #59459 (Add some tests) - #59460 (Include id in Thread's Debug implementation) Failed merges: r? @ghost

rust-highfive assigned nikomatsakis Jan 13, 2019

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jan 13, 2019

petrochenkov force-pushed the turbowarn branch from 5431e7b to 9f66ed6 Compare January 13, 2019 02:10

Centril added I-nominated T-lang Relevant to the language team labels Jan 13, 2019

rfcbot added proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. labels Jan 15, 2019

Centril removed the I-nominated label Jan 17, 2019

This comment has been minimized.

Sign in to view

rfcbot added final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. and removed proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. labels Jan 26, 2019

petrochenkov mentioned this pull request Feb 1, 2019

unnecessary path disambiguator slips past deny(warnings) #58055

Closed

rfcbot added finished-final-comment-period The final comment period is finished for this PR / Issue. and removed final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. labels Feb 5, 2019

Dylan-DPC-zz added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 4, 2019

petrochenkov force-pushed the turbowarn branch from 9f66ed6 to b752f93 Compare March 4, 2019 18:08

petrochenkov added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Mar 4, 2019

This comment has been minimized.

Sign in to view

petrochenkov force-pushed the turbowarn branch from b752f93 to 05b84e5 Compare March 19, 2019 06:39

This comment has been minimized.

Sign in to view

syntax: Remove warning for unnecessary path disambiguators

4b38294

petrochenkov force-pushed the turbowarn branch from 05b84e5 to 4b38294 Compare March 23, 2019 14:20

rust-highfive assigned Centril and unassigned nikomatsakis Mar 26, 2019

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-review Status: Awaiting review from the assignee but also interested parties. labels Mar 26, 2019

SimonSapin mentioned this pull request Mar 26, 2019

Upgrade to rustc 1.35.0-nightly (4c27fb19b 2019-03-25) servo/servo#23102

Merged

Centril mentioned this pull request Mar 27, 2019

Rollup of 13 pull requests #59457

Closed

Centril mentioned this pull request Mar 27, 2019

Rollup of 14 pull requests #59461

Closed

Centril mentioned this pull request Mar 27, 2019

Rollup of 17 pull requests #59466

Closed

cuviper mentioned this pull request Mar 28, 2019

Rollup of 18 pull requests #59471

Merged

bors merged commit 4b38294 into rust-lang:master Mar 28, 2019

petrochenkov deleted the turbowarn branch June 5, 2019 16:33

Uh oh!

Conversation

petrochenkov commented Jan 13, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nikomatsakis commented Jan 15, 2019

Uh oh!

rfcbot commented Jan 15, 2019 • edited by eddyb Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Centril commented Jan 15, 2019

Uh oh!

nikomatsakis commented Jan 17, 2019

Uh oh!

nikomatsakis commented Jan 17, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

petrochenkov commented Jan 17, 2019

Uh oh!

nikomatsakis commented Jan 18, 2019

Uh oh!

nikomatsakis commented Jan 18, 2019

Uh oh!

nikomatsakis commented Jan 18, 2019

Uh oh!

This comment has been minimized.

rfcbot commented Jan 26, 2019

Uh oh!

rfcbot commented Feb 5, 2019

Uh oh!

Dylan-DPC-zz commented Mar 4, 2019

Uh oh!

petrochenkov commented Mar 4, 2019

Uh oh!

This comment has been minimized.

Dylan-DPC-zz commented Mar 18, 2019

Uh oh!

petrochenkov commented Mar 19, 2019

Uh oh!

Dylan-DPC-zz commented Mar 19, 2019

Uh oh!

This comment has been minimized.

Centril commented Mar 26, 2019

Uh oh!

bors commented Mar 26, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

petrochenkov commented Jan 13, 2019 •

edited

Loading

rfcbot commented Jan 15, 2019 •

edited by eddyb

Loading

nikomatsakis commented Jan 17, 2019 •

edited

Loading