-
Notifications
You must be signed in to change notification settings - Fork 13.1k
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
Lower all trivial const paths as ConstArgKind::Path
#135186
base: master
Are you sure you want to change the base?
Changes from all commits
3de14e3
15e9f39
9706c7c
cd21d8c
4dcc15c
e5cf586
b69308e
33c63ae
dd2819f
a912b7d
0862150
8f9b82a
cf5e01e
07254c2
c0ed1dd
25a92ee
7d221d5
84937e1
61c5e74
f8a4a65
abe7d68
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 |
---|---|---|
|
@@ -124,10 +124,11 @@ impl Path { | |
self.segments.first().is_some_and(|segment| segment.ident.name == kw::PathRoot) | ||
} | ||
|
||
/// If this path is a single identifier with no arguments, does not ensure | ||
/// that the path resolves to a const param, the caller should check this. | ||
pub fn is_potential_trivial_const_arg(&self) -> bool { | ||
matches!(self.segments[..], [PathSegment { args: None, .. }]) | ||
// FIXME: add docs | ||
#[tracing::instrument(level = "debug", ret)] | ||
pub fn is_potential_trivial_const_arg(&self, allow_mgca_arg: bool) -> bool { | ||
allow_mgca_arg | ||
|| self.segments.len() == 1 && self.segments.iter().all(|seg| seg.args.is_none()) | ||
} | ||
} | ||
|
||
|
@@ -1177,22 +1178,26 @@ pub struct Expr { | |
} | ||
|
||
impl Expr { | ||
// FIXME: update docs | ||
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. :3 |
||
/// Could this expr be either `N`, or `{ N }`, where `N` is a const parameter. | ||
/// | ||
/// If this is not the case, name resolution does not resolve `N` when using | ||
/// `min_const_generics` as more complex expressions are not supported. | ||
/// | ||
/// Does not ensure that the path resolves to a const param, the caller should check this. | ||
/// This also does not consider macros, so it's only correct after macro-expansion. | ||
pub fn is_potential_trivial_const_arg(&self) -> bool { | ||
pub fn is_potential_trivial_const_arg(&self, allow_mgca_arg: bool) -> bool { | ||
let this = self.maybe_unwrap_block(); | ||
|
||
if let ExprKind::Path(None, path) = &this.kind | ||
&& path.is_potential_trivial_const_arg() | ||
{ | ||
true | ||
if allow_mgca_arg { | ||
matches!(this.kind, ExprKind::Path(..)) | ||
} else { | ||
false | ||
if let ExprKind::Path(None, path) = &this.kind | ||
&& path.is_potential_trivial_const_arg(allow_mgca_arg) | ||
{ | ||
true | ||
} else { | ||
false | ||
} | ||
} | ||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change | ||||||
---|---|---|---|---|---|---|---|---|
|
@@ -1116,7 +1116,8 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { | |||||||
.and_then(|partial_res| partial_res.full_res()) | ||||||||
{ | ||||||||
if !res.matches_ns(Namespace::TypeNS) | ||||||||
&& path.is_potential_trivial_const_arg() | ||||||||
// FIXME: should this only allow single-segment paths? | ||||||||
BoxyUwU marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||
&& path.is_potential_trivial_const_arg(self.tcx.features().min_generic_const_args()) | ||||||||
Comment on lines
+1119
to
+1120
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
Yes I think this should only work for mcg paths rn. Generally this needs to be kept in sync with the logic in name resolution that attempts to resolve type args in both valuens and typens and that currently uses |
||||||||
{ | ||||||||
debug!( | ||||||||
"lower_generic_arg: Lowering type argument as const argument: {:?}", | ||||||||
|
@@ -2083,8 +2084,8 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { | |||||||
) -> &'hir hir::ConstArg<'hir> { | ||||||||
let tcx = self.tcx; | ||||||||
|
||||||||
// FIXME(min_generic_const_args): we only allow one-segment const paths for now | ||||||||
let ct_kind = if path.is_potential_trivial_const_arg() | ||||||||
let ct_kind = if path | ||||||||
.is_potential_trivial_const_arg(tcx.features().min_generic_const_args()) | ||||||||
&& (tcx.features().min_generic_const_args() | ||||||||
|| matches!(res, Res::Def(DefKind::ConstParam, _))) | ||||||||
{ | ||||||||
|
@@ -2094,7 +2095,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { | |||||||
path, | ||||||||
ParamMode::Optional, | ||||||||
AllowReturnTypeNotation::No, | ||||||||
// FIXME(min_generic_const_args): update for `fn foo() -> Bar<FOO<impl Trait>>` support | ||||||||
// FIXME(mgca): update for `fn foo() -> Bar<FOO<impl Trait>>` support | ||||||||
ImplTraitContext::Disallowed(ImplTraitPosition::Path), | ||||||||
None, | ||||||||
); | ||||||||
|
@@ -2158,19 +2159,18 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { | |||||||
}; | ||||||||
let maybe_res = | ||||||||
self.resolver.get_partial_res(expr.id).and_then(|partial_res| partial_res.full_res()); | ||||||||
// FIXME(min_generic_const_args): we only allow one-segment const paths for now | ||||||||
if let ExprKind::Path(None, path) = &expr.kind | ||||||||
&& path.is_potential_trivial_const_arg() | ||||||||
if let ExprKind::Path(qself, path) = &expr.kind | ||||||||
&& path.is_potential_trivial_const_arg(tcx.features().min_generic_const_args()) | ||||||||
&& (tcx.features().min_generic_const_args() | ||||||||
|| matches!(maybe_res, Some(Res::Def(DefKind::ConstParam, _)))) | ||||||||
{ | ||||||||
let qpath = self.lower_qpath( | ||||||||
expr.id, | ||||||||
&None, | ||||||||
qself, | ||||||||
path, | ||||||||
ParamMode::Optional, | ||||||||
AllowReturnTypeNotation::No, | ||||||||
// FIXME(min_generic_const_args): update for `fn foo() -> Bar<FOO<impl Trait>>` support | ||||||||
// FIXME(mgca): update for `fn foo() -> Bar<FOO<impl Trait>>` support | ||||||||
ImplTraitContext::Disallowed(ImplTraitPosition::Path), | ||||||||
None, | ||||||||
); | ||||||||
|
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.
:3