Conversation
rustbot
added
S-waiting-on-author
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
069cb06 to
0816e30
Compare
This comment has been minimized.
This comment has been minimized.
…them as arguments
0816e30 to
74ba4f6
Compare
|
Requested reviewer is already assigned to this pull request. Please choose another assignee. |
rustbot
added
the
S-waiting-on-review
|
cc @rust-lang/rustfmt This PR changes rustc_public cc @oli-obk, @celinval, @ouz-a, @makai410 The parser was modified, potentially altering the grammar of (stable) Rust cc @fmease Changes to the size of AST and/or HIR nodes. cc @nnethercote HIR ty lowering was modified cc @fmease
cc @rust-lang/clippy |
rustbot
removed
the
S-waiting-on-author
| ref ty, | ||
| span, | ||
| default, | ||
| arg_pos: _, |
There was a problem hiding this comment.
What is arg_pos, and does rustfmt need to handle formatting it in some way?
Probably good to add a #![feature(function_arg_const_generics] test case to rustfmt to make sure things are getting formatted as expected.
|
☔ The latest upstream changes (presumably #163306) made this pull request unmergeable. Please resolve the merge conflicts by rebasing. |
There was a problem hiding this comment.
I'm feeling a bit overwhelmed and struggling to review this, and haven't gotten through the parser changes yet. Still though, wanted to comment for what I've seen so far. I think I'd appreciate talking over how to deal with lowering here in zulip or something, I haven't fully thought through the theory space. If you have any direction/resources/etc. on lowering (e.g. whether we fundamentally must lower from hir instead of ast, what potential issues there are from doing so), I'd love to see.
| /// Optional default value for the const generic param. | ||
| default: Option<AnonConst>, | ||
| #[visitable(ignore)] | ||
| arg_pos: Option<u32>, |
There was a problem hiding this comment.
Potentially a better name, but I think especially a doc comment, would go a long way here. It's rather opaque/confusing what this is right now.
|
|
||
| /// Some features require one or more other features to be enabled. | ||
| pub const DEPENDENT_FEATURES: &[(Symbol, &[Symbol])] = &[ | ||
| (sym::function_arg_const_generics, &[sym::min_generic_const_args]), |
There was a problem hiding this comment.
Technically, this is not strictly necessary - it is possible to use direct args right now on stable, plain simple paths to generic parameters are direct args on stable. Maaaybe still want to include it though, probably as being an Or(gca_min_const_items, gca_adts), unsure.
| self.check_param_uses_if_mcg(ct, tcx.hir_span(path_hir_id), false) | ||
| } | ||
|
|
||
| pub fn lower_const_arg_expr(&self, expr: &hir::Expr<'_>, ty: Ty<'tcx>) -> Const<'tcx> { |
There was a problem hiding this comment.
I would strongly prefer this to not be duplicated with the lower_expr_to_const_arg_direct machinery. I'm unsure of how to do so, and is my main bit of feedback in this PR... perhaps it's best to chat about it in Zulip or something. I haven't fully thought this through yet.
This branch has not been deployed
7 participants
This PR adds initial support for argument-position const generics behind the
function_arg_const_genericsfeature gate.It allows const generics to be declared directly in function arguments. The PR adds the basic plumbing across AST, HIR, and
ty, parses these parameters into the function's generics, lowers call arguments into consts. For now, we lower literals, negated literals, const parameters, const items, unit variants, and etc, This works for functions, methods and a couple of more cases.r? @BoxyUwU
cc: @khyperia