-
Notifications
You must be signed in to change notification settings - Fork 13.3k
resolve: Support imports of associated types and glob imports from traits #138712
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
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -2177,7 +2177,13 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ { | |||||
} | ||||||
} | ||||||
Res::Def(DefKind::AssocTy, def_id) => { | ||||||
debug_assert!(path.segments.len() >= 2); | ||||||
if path.segments.len() < 2 { | ||||||
let guar = self | ||||||
.dcx() | ||||||
.struct_span_err(span, "cannot infer type, type annotations needed") | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. #138711 is an issue for reporting some proper error here instead of this stub. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This might seem minor but the phrasing "cannot infer type" isn't quite 'right' in this context since HIR ty lowering doesn't actually use type inference (in the |
||||||
.emit(); | ||||||
return Ty::new_error(tcx, guar); | ||||||
} | ||||||
let _ = self.prohibit_generic_args( | ||||||
path.segments[..path.segments.len() - 2].iter(), | ||||||
GenericsArgsErrExtend::None, | ||||||
|
@@ -2394,7 +2400,13 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ { | |||||
ty::Const::new_unevaluated(tcx, ty::UnevaluatedConst::new(did, args)) | ||||||
} | ||||||
Res::Def(DefKind::AssocConst, did) => { | ||||||
debug_assert!(path.segments.len() >= 2); | ||||||
if path.segments.len() < 2 { | ||||||
let guar = self | ||||||
.dcx() | ||||||
.struct_span_err(span, "cannot infer type, type annotations needed") | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
from what I can tell of There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The error is copypasted from the non- |
||||||
.emit(); | ||||||
return Const::new_error(tcx, guar); | ||||||
} | ||||||
let _ = self.prohibit_generic_args( | ||||||
path.segments[..path.segments.len() - 2].iter(), | ||||||
GenericsArgsErrExtend::None, | ||||||
|
This file was deleted.
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,16 @@ | ||
#![feature(import_trait_associated_functions)] | ||
#![feature(min_generic_const_args)] | ||
#![allow(incomplete_features)] | ||
|
||
trait Trait { | ||
type AssocTy; | ||
const CONST: usize; | ||
} | ||
|
||
use Trait::AssocTy; | ||
type Alias1 = AssocTy; //~ ERROR cannot infer type, type annotations needed | ||
|
||
use Trait::CONST; | ||
type Alias2 = [u8; CONST]; //~ ERROR cannot infer type, type annotations needed | ||
|
||
fn main() {} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,14 @@ | ||
error: cannot infer type, type annotations needed | ||
--> $DIR/import_trait_associated_item_bad.rs:11:15 | ||
| | ||
LL | type Alias1 = AssocTy; | ||
| ^^^^^^^ | ||
|
||
error: cannot infer type, type annotations needed | ||
--> $DIR/import_trait_associated_item_bad.rs:14:20 | ||
| | ||
LL | type Alias2 = [u8; CONST]; | ||
| ^^^^^ | ||
|
||
error: aborting due to 2 previous errors | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,17 @@ | ||
//@ check-pass | ||
|
||
#![feature(import_trait_associated_functions)] | ||
|
||
trait Trait: Default { | ||
fn f() -> Self { Default::default() } | ||
fn g() -> Self { Default::default() } | ||
} | ||
|
||
impl Trait for u8 {} | ||
|
||
use Trait::*; | ||
|
||
fn main() { | ||
let _: u8 = f(); | ||
let _: u8 = g(); | ||
} |
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.
I'm currently strongly leaning towards blocking this PR "on fixing #138711". I.e., implementing this in a slightly more principled manner rather than emitting artificial type inference error diagnostics depending on path segment counts which is quite fragile.
For example while e.g.,
[T; ASSOC_CONST]
andAssocTy
lead to your custom errors getting emitted, a simple change like[T; self::ASSOC_CONST]
andself::AssocTy
respectively — trivially bringing the segment count up to 2 — demonstrates that HIR ty lowering is already perfectly able to deal with such inputs if nudged a bit, emitting errorsambiguous associated constant
andambiguous associated type
respectively (which originate fromprobe_single_bound_for_assoc_item
).Could you experiment with adjusting the necessary lowering methods to enable the existing machinery to handle these sorts of paths? To be totally transparent with you, I haven't thought that much about how much code that would affect.
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.
The fact that the feature import_trait_associated_functions is quite recent (in design and implementation) and — for the AssocConst case — feature min_generic_const_args is still highly experimental and not yet backed by an RFC should be enough to justify my tentative decision.
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.
Ok, I'll look in a few days.