delegation: do not always generate first argument (pt. 1)#156798
delegation: do not always generate first argument (pt. 1)#156798aerooneqq wants to merge 7 commits into
Conversation
303d29a to
c400cb5
Compare
| struct F(S); | ||
| // In glob delegations silently remove first arg if no params or generate default | ||
| // first arg (`arg0`) if it is a static function. | ||
| reuse impl Trait for F { self.0 } |
There was a problem hiding this comment.
Desugaring:
impl Trait for F {
#[attr = Inline(Hint)]
fn value(self: _) -> _ { self.0.value() }
#[attr = Inline(Hint)]
fn r#ref(self: _) -> _ { self.0.r#ref() }
#[attr = Inline(Hint)]
fn mut_ref(self: _) -> _ { self.0.mut_ref() }
#[attr = Inline(Hint)]
fn static_empty() -> _ { Trait::static_empty() }
#[attr = Inline(Hint)]
fn static_one_param(arg0: _) -> _ { Trait::static_one_param(arg0) }
}First arg transformation is not generated even if a static function has first param.
| struct F2(S); | ||
| impl F2 { | ||
| // In list delegations silently remove first arg if it is not a method. | ||
| reuse <S as Trait>::{value, r#ref, mut_ref, static_empty, static_one_param} { self.0 } |
There was a problem hiding this comment.
Desugaring:
#[attr = Inline(Hint)]
fn value(self: _) -> _ { <S as Trait>::value(self.0) }
#[attr = Inline(Hint)]
fn r#ref(self: _) -> _ { <S as Trait>::r#ref(self.0) }
#[attr = Inline(Hint)]
fn mut_ref(self: _) -> _ { <S as Trait>::mut_ref(self.0) }
#[attr = Inline(Hint)]
fn static_empty() -> _ { <S as Trait>::static_empty() }
#[attr = Inline(Hint)]
fn static_one_param(arg0: _) -> _ { <S as Trait>::static_one_param(arg0) }First arg transformation is not generated even if a static function has first param.
| reuse to_reuse::one_param { self + 1 } | ||
|
|
||
| // In list delegations silently remove first arg if there are no params. | ||
| reuse to_reuse::{empty as empty1, one_param as one_param1} { self + 1 } |
There was a problem hiding this comment.
Desugaring:
// In list delegations silently remove first arg if there are no params.
#[attr = Inline(Hint)]
fn empty1() -> _ { to_reuse::empty() }
#[attr = Inline(Hint)]
fn one_param1(arg0: _) -> _ { to_reuse::one_param(self + 1) }We generated first arg transformation as it is a free function.
c400cb5 to
3983a7b
Compare
|
|
||
| struct F3(S); | ||
| impl Trait for F3 { | ||
| reuse trait_to_reuse::{value, r#ref, mut_ref, static_empty, static_one_param} { self.0 } |
There was a problem hiding this comment.
Desugaring:
#[attr = Inline(Hint)]
fn value(self: _) -> _ { trait_to_reuse::value(self.0) }
#[attr = Inline(Hint)]
fn r#ref(self: _) -> _ { trait_to_reuse::r#ref(self.0) }
#[attr = Inline(Hint)]
fn mut_ref(self: _) -> _ { trait_to_reuse::mut_ref(self.0) }
#[attr = Inline(Hint)]
fn static_empty() -> _ { trait_to_reuse::static_empty() }
#[attr = Inline(Hint)]
fn static_one_param(arg0: _)
-> _ { trait_to_reuse::static_one_param(arg0) }
//~^ ERROR: mismatched types
//~| ERROR: mismatched typesself resolves to F3.
|
|
||
| struct F4(S); | ||
| impl F4 { | ||
| reuse trait_to_reuse::{value, r#ref, mut_ref, static_empty, static_one_param} { self.0 } |
There was a problem hiding this comment.
Desugaring:
#[attr = Inline(Hint)]
fn value<impl Trait>(arg0: _) -> _ { trait_to_reuse::value(self.0) }
#[attr = Inline(Hint)]
fn r#ref<impl Trait>(arg0: _) -> _ { trait_to_reuse::r#ref(self.0) }
#[attr = Inline(Hint)]
fn mut_ref<impl Trait>(arg0: _) -> _ { trait_to_reuse::mut_ref(self.0) }
#[attr = Inline(Hint)]
fn static_empty() -> _ { trait_to_reuse::static_empty() }
#[attr = Inline(Hint)]
fn static_one_param(arg0: _)
-> _ { trait_to_reuse::static_one_param(self.0) }self resolves to first argument, as it is a plain impl and sig_id resolves to free functions not to a trait method in contrast to case with F3. We generated block for static_one_param as it is a free function, in F3 it is an associated function so we generate arg0.
|
The parser was modified, potentially altering the grammar of (stable) Rust cc @fmease |
|
I think the solution for the dead code problem here brings much worse problems than the initial problem itself. If the target expression is unused, we can generate something like instead of messing with the definition hierarchy. @rustbot author |
|
Reminder, once the PR becomes ready for a review, use |
|
Ah, the question is what |
Although leaving e.g. |
|
I just realized that removing code here is a future compatibility hazard. reuse {foo} { std::something_unstable() };If |
|
Also, removing or not removing the target expression depending on whether the delegation is a part of the list or not is probably not a good language design. // now do two different things.
reuse foo;
reuse {foo};I think we'll eventually need to remove the target expression either always, or always for associated functions. |
5200e14 to
ee1c893
Compare
This comment has been minimized.
This comment has been minimized.
da21a60 to
eb94990
Compare
|
I want to split this work into 4 parts:
|
The plan sounds good. |
| let expansion = self.parent_scope.expansion; | ||
| self.r.define_local(parent, ident, ns, self.res(def_id), vis, item.span, expansion); | ||
| } else if !matches!(&item.kind, AssocItemKind::Delegation(deleg) if deleg.from_glob) | ||
| } else if !matches!(&item.kind, AssocItemKind::Delegation(d) if matches!(d.source, DelegationSource::Glob)) |
There was a problem hiding this comment.
| } else if !matches!(&item.kind, AssocItemKind::Delegation(d) if matches!(d.source, DelegationSource::Glob)) | |
| } else if let AssocItemKind::Delegation(d) = &item.kind && d.source == DelegationSource::Glob |
| item: &'a ast::Item<Node::ItemKind>, | ||
| suffixes: &'a [(Ident, Option<Ident>)], | ||
| item_span: Span, | ||
| from_glob: bool, |
There was a problem hiding this comment.
| source: DelegationSource, |
| && self | ||
| .resolver | ||
| .delegation_info(self.owner.def_id) | ||
| .is_some_and(|i| i.block_contains_defs) |
There was a problem hiding this comment.
Would it possible to detect this post-factum, instead of tracking things in advance through def collector?
Like add a small AST visitor to visit delegation.body and check if there's anything with definitions?
This visitor would only run for delegations where performance is not much of a concern at the moment.
This would avoid polluting the general-purpose infra like def collector with delegation-specific concerns.
|
|
||
| // Report an error if user has explicitly specified delegation's target expression | ||
| // in a single delegation when reused function has no params. | ||
| if param_count == 0 && matches!(delegation.source, DelegationSource::Single) { |
There was a problem hiding this comment.
| if param_count == 0 && matches!(delegation.source, DelegationSource::Single) { | |
| if param_count == 0 && should_generate_block { |
Otherwise we are missing e.g. a free function with zero params in a list.
| let (param_count, c_variadic) = self.param_count(sig_id); | ||
|
|
||
| if !self.check_block_soundness(delegation, sig_id, is_method, param_count) { | ||
| return self.generate_delegation_error(span, delegation); |
There was a problem hiding this comment.
Similarly to #157248 (comment) this is sub-optimal from error recovery / diagnostics point of view, but that's not a first concern right now.
View all comments
This PR supports not generating delegation's "dead code" - situation when there is no parameters in a source function or we expanded a reuse of a static function without self, in this case we do not have to generate delegation's block even if the user has explicitly specified it.
We generate block:
Fixes #154427. Part of #118212.
r? @petrochenkov