Skip to content
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

Replace #[rustc_args_required_const(n)] with per-argument attr #69282

Closed
ecstatic-morse opened this issue Feb 19, 2020 · 9 comments
Closed

Replace #[rustc_args_required_const(n)] with per-argument attr #69282

ecstatic-morse opened this issue Feb 19, 2020 · 9 comments
Labels
A-attributes Area: #[attributes(..)] C-cleanup Category: PRs that clean code up or issues documenting cleanup. C-enhancement Category: An issue proposing an enhancement or a PR with one. E-medium Call for participation: Medium difficulty. Experience needed to fix: Intermediate. F-param_attrs `#![feature(param_attrs)]`

Comments

@ecstatic-morse
Copy link
Contributor

ecstatic-morse commented Feb 19, 2020

RFC 2565, which was recently stabilized, allows individual function parameters to have attributes. One of the motivating examples for that RFC was the ugliness of #[rustc_args_required_const], which specifies the const argument(s) by index. We should transition away from #[rustc_args_required_const] to a per-argument attribute.

@petrochenkov has proposed #[rustc_required_const]. These attributes are perma-unstable so there are no backwards compatibility concerns.

@ecstatic-morse ecstatic-morse added A-attributes Area: #[attributes(..)] C-feature-request Category: A feature request, i.e: not implemented / a PR. F-param_attrs `#![feature(param_attrs)]` E-medium Call for participation: Medium difficulty. Experience needed to fix: Intermediate. labels Feb 19, 2020
@Centril Centril added C-cleanup Category: PRs that clean code up or issues documenting cleanup. C-enhancement Category: An issue proposing an enhancement or a PR with one. and removed C-feature-request Category: A feature request, i.e: not implemented / a PR. labels Feb 20, 2020
@petrochenkov
Copy link
Contributor

petrochenkov commented Feb 26, 2020

[triagebot] 👍

EDIT: I'm not attached to the mentioned naming.

@jakevossen5
Copy link
Contributor

jakevossen5 commented Feb 26, 2020

I would be willing to take this up. Are there any mentors out there who could be available if I have questions?

@rustbot claim

@LeSeulArtichaut
Copy link
Contributor

@jakevossen5 Are you still working on this? Or can I take it up?

@jakevossen5
Copy link
Contributor

@LeSeulArtichaut , Apologies, I have gotten a bit busy in the past week. I am still working on this and hopefully have a PR up sometime next week.

@eddyb
Copy link
Member

eddyb commented Mar 22, 2020

Ironically, the benefit from this is pretty minor, and it'd be trickier to implement than today, because the param attributes aren't kept cross-crate, so they need to be encoded some other way.

Probably a decent way would be to add a Lazy<BitSet> to this, I guess?

struct FnData {
asyncness: hir::IsAsync,
constness: hir::Constness,
param_names: Lazy<[ast::Name]>,
}

@jakevossen5
Copy link
Contributor

@eddyb Do you mean a new element of the struct, like param_consts that keeps track of which are constants? Do I understand that correctly?

@eddyb
Copy link
Member

eddyb commented Apr 10, 2020

I think we should resolve #70271 first before touching #[rustc_args_required_const] further.

@jakevossen5
Copy link
Contributor

Sounds good. I will watch that thread and when it it closed circle back to here.

@Alexendoo
Copy link
Member

rustc_args_required_const has been removed - #85110

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-attributes Area: #[attributes(..)] C-cleanup Category: PRs that clean code up or issues documenting cleanup. C-enhancement Category: An issue proposing an enhancement or a PR with one. E-medium Call for participation: Medium difficulty. Experience needed to fix: Intermediate. F-param_attrs `#![feature(param_attrs)]`
Projects
None yet
Development

No branches or pull requests

7 participants