Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

slightly better deprecation message for fn syntax #19020

Merged
merged 1 commit into from
Nov 18, 2014

Conversation

Gankra
Copy link
Contributor

@Gankra Gankra commented Nov 17, 2014

Came up on IRC that this was a bit unhelpful as to what should actually be done. I am new to changing compiler messages, please let me know if there's anything else that needs to be done to accomadate this change.

(My build system is still constantly crashing [Is bors contagious?], so this hasn't been formally checked. I figure it's a simple enough change that any consequences [like compile-fail expected messages?] can be eyeballed by someone more experienced.)

@@ -1229,7 +1229,8 @@ impl<'a> Parser<'a> {
{
if self.eat(&token::Lt) {
if lifetime_defs.is_empty() {
self.warn("deprecated syntax, use `for` keyword now");
self.warn("deprecated syntax, use `for` keyword now \
(e.g.,change `fn<'a>` to `for<'a> fn`)");
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: this comma probably needs a space after it.

(Also, maybe the other comma (on the line above) should be changed to a semicolon?)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

semicolon sounds good. I would probably just kill the comma after e.g. upon reflection.

@Gankra
Copy link
Contributor Author

Gankra commented Nov 17, 2014

@P1start cleaned the message up

@alexcrichton
Copy link
Member

Are closures also covered by this warning? For example:

type A = <'a>|&'a int| -> &'a int;

Just wanna make sure we're not recommending fn syntax for a closure.

@Gankra
Copy link
Contributor Author

Gankra commented Nov 17, 2014

I honestly have no idea. It seems to be the only deprecation notice for using the old syntax, but I'm unclear as to the scope that new syntax applies to.

@alexcrichton
Copy link
Member

All I'm really getting at is that this same message is used for one of two locations for the deprecated syntax, and the suggested fix only applies to one of the locations:

type A = <'a> ||;
type B = fn<'a>();
$ rustc foo.rs
foo.rs:1:11: 1:13 warning: deprecated syntax, use `for` keyword now
foo.rs:1 type A = <'a> ||;
                   ^~
foo.rs:2:13: 2:15 warning: deprecated syntax, use `for` keyword now
foo.rs:2 type B = fn<'a>();
                     ^~

@Gankra
Copy link
Contributor Author

Gankra commented Nov 17, 2014

The form of the fix is that same, right? <'a>|| -> for<'a>||? It is an ``e.g.`, so it doesn't strictly claim to be exhaustive. I can update it to be exhaustive, if that's what you're looking for.

@alexcrichton
Copy link
Member

hm ok I guess this is ok for now with the "e.g." in front, sorry for the confusion!

@bors bors merged commit 8c467f7 into rust-lang:master Nov 18, 2014
lnicola pushed a commit to lnicola/rust that referenced this pull request Jan 27, 2025
fix: Prevent infinite recursion of bounds formatting
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants