Implement generic const items #113522

fmease · 2023-07-10T07:15:41Z

This implements generic parameters and where-clauses on free and associated const items under the experimental feature gate generic_const_items. See rust-lang/lang-team#214.

Tracking issue: #113521.
Fixes #104400.

This PR doesn't include rustfmt support as per nightly style procedure and it doesn't add rustdoc JSON support (see related Zulip topic).

CC @scottmcm @compiler-errors @WalterSmuts
r? @oli-obk

Resolved Questions

Q: Should const ADD<const N: usize, const M: usize>: usize = N + M; ADD::<0, 1> trigger the error generic parameters may not be used in const operations (which can be unlocked with #![feature(generic_const_exprs)]). Currently it doesn't. Or does this fall under this paragraph?
- A: No, Implement generic const items #113522 (comment)
Q: Should const UNUSED: () = () where String: Copy; (with #![feature(trivial_bounds)] and with UNUSED unused) succeed compilation? Currently it doesn't: evaluation of constant value failed // entering unreachable code
- A: Yes, but postponed until stabilization (latest), Implement generic const items #113522 (comment)

compiler/rustc_hir_analysis/src/check/check.rs

tests/ui/generic-consts/associated-const-equality.rs

lcnr · 2023-07-10T10:03:45Z

some thoughts without looking at the PR:

please rename the feature gate to generic_const_items because generic_consts is too general and therefore confusing
while I am strongly in favor of this feature, I would like this work to go through the proper lang/types team procedures. Afaict @scottmcm mentioned that they would second a lang proposal for them, so please open a lang MCP (I think that's how t-lang currently does stuff)
I would you to potentially split this into multiple PRs, implementing parsing + nameres first and then implementing type system support in a separate PR. I don't think we have a single expert able to sufficiently review everything here and would like to avoid having multiple reviewers with overlapping responsibilities here. It should also avoid some merge conflicts hopefully

Should const ADD<const N: usize, const M: usize>: usize = N + M; ADD::<0, 1> trigger the error generic parameters may not be used in const operations (which can be unlocked with #![feature(generic_const_exprs)]). Currently it doesn't. Or does this fall under this paragraph?

It should not, ADD::<0, 1> does not depend on any generic parameters itself. THis is similar to <() as TraitAdd<0, 1>>::ASSOC which is already allowed.

Should const UNUSED: () = () where String: Copy; (with #![feature(trivial_bounds)] and with UNUSED unused) succeed compilation? Currently it doesn't: evaluation of constant value failed // entering unreachable code

ideally yes, but I think this is something with can deal with later. Const evaluation with invalid bounds is generally somewhat meh rn and not something you have to deal with in this PR.

cjgillot

Some preliminary remarks on this PR.

I disagree with @lcnr about splitting into multiple PRs. Having a single PR makes it much easier to check the global consistency of the implementation. The PR is already neatly split into consistent commits, which makes this practical. We can have multiple assigned reviewers on this single PR + 1 coordinating the review.

I'm available for the AST+HIR+nameres part.

compiler/rustc_ast_passes/src/ast_validation.rs

compiler/rustc_ast_lowering/src/item.rs

compiler/rustc_resolve/src/late.rs

tests/ui/suggestions/missing-lifetime-in-assoc-const-type.stderr

oli-obk · 2023-07-10T13:54:53Z

It was my understanding that adding a new feature gate only needs a lang team member who's ok with it, not a lang team FCP (MCPs have been discontinued). I also think that this feature is ok to review in one PR, but I will likely ask some general things to happen in other PRs and this one to be rebased over them.

compiler/rustc_hir_analysis/src/check/check.rs

compiler/rustc_hir_analysis/src/check/compare_impl_item.rs

compiler/rustc_hir_analysis/src/check/wfcheck.rs

compiler/rustc_hir_analysis/src/collect.rs

compiler/rustc_ast/src/visit.rs

compiler/rustc_ast_pretty/src/pprust/state/item.rs

compiler/rustc_parse/src/parser/item.rs

bors · 2023-07-13T19:19:32Z

☔ The latest upstream changes (presumably #113646) made this pull request unmergeable. Please resolve the merge conflicts.

compiler/rustc_resolve/src/late.rs

compiler/rustc_hir_analysis/src/check/wfcheck.rs

compiler/rustc_resolve/src/late.rs

compiler/rustc_resolve/src/late/diagnostics.rs

oli-obk · 2023-07-17T13:56:49Z

tests/ui/generic-const-items/const-trait-impl.rs

+const CREATE<T: ~const Create>: T = T::create();
+
+pub const K0: i32 = CREATE::<i32>;
+pub const K1: i32 = CREATE; // arg inferred
+
+#[const_trait]
+trait Create {
+    fn create() -> Self;
+}
+
+impl const Create for i32 {
+    fn create() -> i32 {
+        4096
+    }
+}


impl Create for u32 { fn create() -> u32 { println!("foobar"); 42 } }

will probably still compile (modulo the inference example), and then die in const eval. We'll need to interpret ~const (or just const) bounds as always-const, instead of maybe-const in const items. This can happen in a follow up though.

cc @fee1-dead

Ooh this is interesting. This should probably happen during astconv when ~const gets desugared into generic params for the trait. And in this case the generic param should just be host = false (disabling the host effect thus making this a const trait bound)

I think this would fit nicely into the PR where constness is removed from TraitPredicate.

tests/ui/generic-const-items/inference-failure.rs

fmease · 2023-07-28T21:00:10Z

Rebased, CI green & none of the PRs rolled up in #114181 (pending) should lead to any kind of conflict 🤞
Could sb. reapprove? Thanks in advance! <3

compiler-errors · 2023-07-28T21:01:07Z

@bors r=cjgillot

bors · 2023-07-28T21:01:09Z

📌 Commit 203d400 has been approved by cjgillot

It is now in the queue for this repository.

bors · 2023-07-28T22:04:29Z

⌛ Testing commit 203d400 with merge 04abc37...

bors · 2023-07-28T23:53:06Z

☀️ Test successful - checks-actions
Approved by: cjgillot
Pushing 04abc37 to master...

rust-timer · 2023-07-29T05:32:07Z

Finished benchmarking commit (04abc37): comparison URL.

Overall result: ❌ regressions - ACTION NEEDED

Next Steps: If you can justify the regressions found in this perf run, please indicate this with @rustbot label: +perf-regression-triaged along with sufficient written justification. If you cannot justify the regressions please open an issue or create a new PR that fixes the regressions, add a comment linking to the newly created issue or PR, and then add the perf-regression-triaged label to this PR.

@rustbot label: +perf-regression
cc @rust-lang/wg-compiler-performance

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	0.7%	[0.3%, 1.0%]	17
Regressions ❌ (secondary)	0.9%	[0.3%, 1.4%]	9
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	0.7%	[0.3%, 1.0%]	17

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	2.1%	[1.6%, 3.0%]	4
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-2.4%	[-2.5%, -2.3%]	2
All ❌✅ (primary)	-	-	0

Cycles

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-3.5%	[-5.2%, -1.8%]	6
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-3.5%	[-5.2%, -1.8%]	6

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 649.861s -> 649.942s (0.01%)

nnethercote · 2023-07-29T08:23:18Z

@fmease: a few perf regressions there, is that expected? Any ideas how to mitigate them?

fmease · 2023-07-29T08:48:19Z

The regressions are somewhat expected (we're doing more work per const item). While I've expected a benchmark like many-assoc-items to regress, I'm surprised that perf is worse for free const items as well (libc contains a number of them).

I'm looking into it. Maybe we can short-circuit in some places.

rylev · 2023-08-05T13:03:45Z

@rustbot label: +perf-regression-triaged

Do not format generic consts We introduced **nightly support** for generic const items in rust-lang#113522, but formatting of consts was not modified. Making them format *correctly* is hard, so let's just bail formatting them so we don't accidentally strip their generics and where clauses. This is essentially no-op formatting for generic const items. r? `@calebcartwright` or `@ytmimi`

Do not format generic consts We introduced **nightly support** for generic const items in rust-lang#113522, but formatting of consts was not modified. Making them format *correctly* is hard, so let's just bail formatting them so we don't accidentally strip their generics and where clauses. This is essentially no-op formatting for generic const items. r? ``@calebcartwright`` or ``@ytmimi``

Do not format generic consts We introduced **nightly support** for generic const items in rust-lang#113522, but formatting of consts was not modified. Making them format *correctly* is hard, so let's just bail formatting them so we don't accidentally strip their generics and where clauses. This is essentially no-op formatting for generic const items. r? ```@calebcartwright``` or ```@ytmimi```

Do not format generic consts We introduced **nightly support** for generic const items in rust-lang#113522, but formatting of consts was not modified. Making them format *correctly* is hard, so let's just bail formatting them so we don't accidentally strip their generics and where clauses. This is essentially no-op formatting for generic const items. r? ````@calebcartwright```` or ````@ytmimi````

Do not format generic consts We introduced **nightly support** for generic const items in rust-lang#113522, but formatting of consts was not modified. Making them format *correctly* is hard, so let's just bail formatting them so we don't accidentally strip their generics and where clauses. This is essentially no-op formatting for generic const items. r? `````@calebcartwright````` or `````@ytmimi`````

Rollup merge of rust-lang#132540 - compiler-errors:gc, r=calebcartwright Do not format generic consts We introduced **nightly support** for generic const items in rust-lang#113522, but formatting of consts was not modified. Making them format *correctly* is hard, so let's just bail formatting them so we don't accidentally strip their generics and where clauses. This is essentially no-op formatting for generic const items. r? `````@calebcartwright````` or `````@ytmimi`````

rustbot assigned oli-obk Jul 10, 2023

fmease mentioned this pull request Jul 10, 2023

Tracking issue for generic const items #113521

Open

7 tasks

This comment has been minimized.

Sign in to view

fmease force-pushed the generic-consts branch from ec34a70 to ee43a02 Compare July 10, 2023 07:34

Noratrieb reviewed Jul 10, 2023

View reviewed changes

compiler/rustc_hir_analysis/src/check/check.rs Outdated Show resolved Hide resolved

fmease force-pushed the generic-consts branch from ee43a02 to bef692d Compare July 10, 2023 07:42

fmease commented Jul 10, 2023

View reviewed changes

tests/ui/generic-consts/associated-const-equality.rs Outdated Show resolved Hide resolved

rustbot removed the A-rustdoc-json Area: Rustdoc JSON backend label Jul 10, 2023

lcnr added the needs-fcp This change is insta-stable, so needs a completed FCP to proceed. label Jul 10, 2023

fmease changed the title ~~Implement generic consts~~ Implement generic const items Jul 10, 2023

cjgillot reviewed Jul 10, 2023

View reviewed changes

compiler-errors reviewed Jul 10, 2023

View reviewed changes

cjgillot self-assigned this Jul 10, 2023

fmease force-pushed the generic-consts branch from bef692d to 20228ec Compare July 11, 2023 17:25

rustbot added the A-rustdoc-json Area: Rustdoc JSON backend label Jul 11, 2023

cjgillot reviewed Jul 12, 2023

View reviewed changes

fmease force-pushed the generic-consts branch from 20228ec to 55933bc Compare July 14, 2023 20:37

This comment has been minimized.

Sign in to view

fmease force-pushed the generic-consts branch 2 times, most recently from 53d06c1 to 4465212 Compare July 16, 2023 19:18

oli-obk requested changes Jul 17, 2023

View reviewed changes

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 28, 2023

bors added the merged-by-bors This PR was explicitly merged by bors. label Jul 28, 2023

bors merged commit 04abc37 into rust-lang:master Jul 28, 2023
11 checks passed

rustbot added this to the 1.73.0 milestone Jul 28, 2023

fmease deleted the generic-consts branch July 28, 2023 23:53

This was referenced Jul 29, 2023

Make privacy visitor use types more (instead of HIR) #113671

Merged

Rustdoc small cleanups #114129

Merged

Rollup of 8 pull requests #114184

Closed

rustbot added the perf-regression Performance regression. label Jul 29, 2023

rustbot added the F-generic_const_items `#![feature(generic_const_items)]` label Jul 29, 2023

rustbot added the perf-regression-triaged The performance regression has been triaged. label Aug 5, 2023

fmease mentioned this pull request Sep 10, 2023

Do not suggest "using a local generic parameter" for constants #115720

Closed

matthiaskrgr mentioned this pull request Feb 11, 2024

ICE: const parameter types cannot be generic #120941

Closed

compiler-errors mentioned this pull request Nov 2, 2024

Do not format generic consts #132540

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement generic const items #113522

Implement generic const items #113522

fmease commented Jul 10, 2023 •

edited

Loading

This comment has been minimized.

lcnr commented Jul 10, 2023

cjgillot left a comment

oli-obk commented Jul 10, 2023

bors commented Jul 13, 2023

This comment has been minimized.

oli-obk Jul 17, 2023

fee1-dead Jul 17, 2023

fmease commented Jul 28, 2023

compiler-errors commented Jul 28, 2023

bors commented Jul 28, 2023

bors commented Jul 28, 2023

bors commented Jul 28, 2023

rust-timer commented Jul 29, 2023

nnethercote commented Jul 29, 2023

fmease commented Jul 29, 2023 •

edited

Loading

rylev commented Aug 5, 2023

Implement generic const items #113522

Implement generic const items #113522

Conversation

fmease commented Jul 10, 2023 • edited Loading

This comment has been minimized.

lcnr commented Jul 10, 2023

cjgillot left a comment

Choose a reason for hiding this comment

oli-obk commented Jul 10, 2023

bors commented Jul 13, 2023

This comment has been minimized.

oli-obk Jul 17, 2023

Choose a reason for hiding this comment

fee1-dead Jul 17, 2023

Choose a reason for hiding this comment

fmease commented Jul 28, 2023

compiler-errors commented Jul 28, 2023

bors commented Jul 28, 2023

bors commented Jul 28, 2023

bors commented Jul 28, 2023

rust-timer commented Jul 29, 2023

Overall result: ❌ regressions - ACTION NEEDED

Instruction count

Max RSS (memory usage)

Cycles

Binary size

nnethercote commented Jul 29, 2023

fmease commented Jul 29, 2023 • edited Loading

rylev commented Aug 5, 2023

fmease commented Jul 10, 2023 •

edited

Loading

fmease commented Jul 29, 2023 •

edited

Loading