-
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
in which we decline to suggest the anonymous lifetime in declarations #61679
in which we decline to suggest the anonymous lifetime in declarations #61679
Conversation
src/librustc/hir/lowering.rs
Outdated
if let hir::TyKind::TraitObject(..) = ty.node { | ||
self.maybe_lint_bare_trait(t.span, t.id, qself.is_none() && path.is_global()); | ||
} | ||
let ty = self.lower_path_ty(t, qself, path, ParamMode::Explicit, itctx); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: directly return here instead of using an intermediate variable
); | ||
P(t) | ||
} else { | ||
self.lower_ty(&f.ty, ImplTraitContext::disallowed()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
alternatively you could add a ParamMode
argument to lower_ty
, not sure if that is better though
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That was my first idea, but I decided against it, because it seemed inelegant to change the signature when most of the TyKind
match arm bodies don't concern themselves with ParamMode
.
r=me with nits addressed |
The elided-lifetimes-in-path lint (part of our suite of Rust 2018 idiom lints which we are hoping to promote to Warn status) was firing with an illegal suggestion to write an anonymous lifetime in a struct/item declaration (where we don't allow it). The linting code was already deciding whether to act on the basis of a `ParamMode` enum, indicating whether the present path-segment was part of an expression, or anywhere else. The present case seemed to be part of the "anywhere else", and yet meriting different rules as far as the lint was concerned, so it seemed expedient to introduce a new enum member. We yank out a `TyKind::Path` arm into its own method so that we can call it with our new `ParamMode` specifically when lowering struct fields. (The alternative strategy of changing the signature of `lower_ty` to take a `ParamMode` would be inelegant given that most of the `TyKind` match arm bodies therein don't concern themselves with `ParamMode`.) Resolves rust-lang#61124.
9c55aa0
to
7a3184a
Compare
@bors r=oli-obk |
📌 Commit 7a3184a has been approved by |
…ifetime, r=oli-obk in which we decline to suggest the anonymous lifetime in declarations The elided-lifetimes-in-path lint (part of our suite of Rust 2018 idiom lints which we are hoping to promote to Warn status) was firing with an illegal suggestion to write an anonymous lifetime in a struct/item declaration (where we don't allow it). The linting code was already deciding whether to act on the basis of a `ParamMode` enum, indicating whether the present path-segment was part of an expression, or anywhere else. The present case seemed to be part of the "anywhere else", and yet meriting different rules as far as the lint was concerned, so it seemed expedient to introduce a new enum member. We yank out `TyKind::Path` arm into its own method so that we can call it with our new `ParamMode` specifically when lowering struct fields—one would have hoped to think of something more elegant than this, but it definitely beats changing the signature of `lower_ty` to take a `ParamMode`! Resolves #61124. cc @memoryruins r? @oli-obk
☀️ Test successful - checks-travis, status-appveyor |
…fmease Remove `ParamMode::ExplicitNamed` This was introduced as a hack to improve a diagnostics suggestion in rust-lang#61679. It was subsequently broken, but also it was an incomplete hack that I don't believe we need to support, so let's just remove it.
…fmease Remove `ParamMode::ExplicitNamed` This was introduced as a hack to improve a diagnostics suggestion in rust-lang#61679. It was subsequently broken, but also it was an incomplete hack that I don't believe we need to support, so let's just remove it.
Rollup merge of rust-lang#129626 - compiler-errors:explicit-named, r=fmease Remove `ParamMode::ExplicitNamed` This was introduced as a hack to improve a diagnostics suggestion in rust-lang#61679. It was subsequently broken, but also it was an incomplete hack that I don't believe we need to support, so let's just remove it.
The elided-lifetimes-in-path lint (part of our suite of Rust 2018 idiom lints which we are hoping to promote to Warn status) was firing with an illegal suggestion to write an anonymous lifetime in a
struct/item declaration (where we don't allow it). The linting code was already deciding whether to act on the basis of a
ParamMode
enum, indicating whether the present path-segment was part of anexpression, or anywhere else. The present case seemed to be part of the "anywhere else", and yet meriting different rules as far as the lint was concerned, so it seemed expedient to introduce a new enum member. We yank out
TyKind::Path
arm into its own method so that we can call it with our newParamMode
specifically when lowering struct fields—one would have hoped to think of something more elegant than this, but it definitely beats changing the signature oflower_ty
to take aParamMode
!Resolves #61124.
cc @memoryruins
r? @oli-obk