Deny more `~const` trait bounds #117817

fmease · 2023-11-11T14:10:15Z

thereby fixing a family of ICEs (delayed bugs) for feature(const_trait_impl, effects) code.

As discussed
r? @fee1-dead

matthiaskrgr · 2023-11-11T15:02:25Z

Does this fix any of these? :D
https://github.com/rust-lang/rust/issues?q=is%3Aissue+is%3Aopen+author%3Amatthiaskrgr+effects

fee1-dead

personally speaking I'd rather make Some(DisalloweTildeConstContext) the default

sure, let's do that!

compiler/rustc_ast_passes/src/ast_validation.rs

fmease · 2023-11-11T17:38:41Z

Does this fix any of these? :D

No, it doesn't :|

fmease · 2023-11-12T00:06:13Z

compiler/rustc_ast_passes/src/ast_validation.rs

+
+                self.visit_vis(&item.vis);
+                self.visit_ident(item.ident);
+                self.with_tilde_const(None, |this| this.visit_generics(generics));


Currently permitting ~const trait bounds on inherent impls not to regress rfc-2632-const-trait-impl/tilde_const_on_impl_bound.rs. Not sure if the following code should compile at some point:

struct Struct; #[const_trait] trait Trait {} impl<T: ~const Trait> Struct { const fn f(_: T) {} }

If I were to straight up deny ~const here, it'd obviously fix #117004. I haven't tested yet if the ~const lowering code can properly lower the code above.

We should probably deny this first. We don't really need this to work and making it work certainly adds to the overhead. Perhaps a followup PR? Or I can do that. Thanks for working on this! :)

I think we should deny ~const here. If we want to be able to use Trait in const fn f, I think the user should need to write:

struct Struct; #[const_trait] trait Trait {} impl<T: Trait> Struct { const fn f(_: T) where T: ~const Trait {} }

at least in my opinion.

fee1-dead · 2023-11-12T04:39:21Z

compiler/rustc_ast_passes/messages.ftl

+    .trait = this trait is not a `#[const_trait]`, so it cannot have `~const` trait bounds
+    .impl = this impl is not `const`, so it cannot have `~const` trait bounds
+    .object = trait objects cannot have `~const` trait bounds
+    .item = this item cannot have `~const` trait bounds


This is quite ambiguous and we'd probably always want to have a reason.. OTOH, we don't want to allow tilde const for these "unknown" cases. Would be nice if this was turned into a span_bug! so that a more specific context is always defined.

No need for this PR to have this though.

fee1-dead · 2023-11-12T04:39:32Z

@bors r+

bors · 2023-11-12T04:39:35Z

📌 Commit 8ce5d78 has been approved by fee1-dead

It is now in the queue for this repository.

bors · 2023-11-12T04:40:47Z

⌛ Testing commit 8ce5d78 with merge a04d56b...

bors · 2023-11-12T06:36:29Z

☀️ Test successful - checks-actions
Approved by: fee1-dead
Pushing a04d56b to master...

rust-timer · 2023-11-12T08:04:32Z

Finished benchmarking commit (a04d56b): comparison URL.

Overall result: ❌ regressions - no action needed

@rustbot label: -perf-regression

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
Regressions ❌ (secondary)	2.0%	[2.0%, 2.0%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-	-	0

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.5%	[0.5%, 0.5%]	1
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-0.8%	[-0.8%, -0.8%]	1
Improvements ✅ (secondary)	-0.5%	[-0.5%, -0.5%]	1
All ❌✅ (primary)	-0.2%	[-0.8%, 0.5%]	2

Cycles

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

Binary size

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

Bootstrap: 674.671s -> 675.185s (0.08%)
Artifact size: 311.12 MiB -> 311.12 MiB (0.00%)

fmease · 2023-11-12T08:39:41Z

perf regression looks kinda spurious

…headers, r=fee1-dead Deny `~const` trait bounds in inherent impl headers Follow-up to rust-lang#117817. Implements rust-lang#117817 (comment). Fixes rust-lang#117004. r? fee1-dead or compiler

Rollup merge of rust-lang#119059 - fmease:no-tilde-const-in-inh-impl-headers, r=fee1-dead Deny `~const` trait bounds in inherent impl headers Follow-up to rust-lang#117817. Implements rust-lang#117817 (comment). Fixes rust-lang#117004. r? fee1-dead or compiler

…r=fee1-dead Deny `~const` trait bounds in inherent impl headers Follow-up to #117817. Implements rust-lang/rust#117817 (comment). Fixes #117004. r? fee1-dead or compiler

rustbot assigned fee1-dead Nov 11, 2023

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Nov 11, 2023

fmease added F-const_trait_impl `#![feature(const_trait_impl)]` and removed T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 11, 2023

fmease mentioned this pull request Nov 11, 2023

Properly reject default on free const items #117818

Merged

fee1-dead reviewed Nov 11, 2023

View reviewed changes

compiler/rustc_ast_passes/src/ast_validation.rs Outdated Show resolved Hide resolved

fmease added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Nov 11, 2023

fmease force-pushed the deny-more-tilde-const branch from e345737 to 2fe9827 Compare November 11, 2023 17:34

fmease added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Nov 11, 2023

Deny more ~const trait bounds

8ce5d78

fmease force-pushed the deny-more-tilde-const branch from 2fe9827 to 8ce5d78 Compare November 11, 2023 23:00

fmease commented Nov 12, 2023

View reviewed changes

fee1-dead reviewed Nov 12, 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 Nov 12, 2023

bors added the merged-by-bors This PR was explicitly merged by bors. label Nov 12, 2023

bors merged commit a04d56b into rust-lang:master Nov 12, 2023
12 checks passed

rustbot added this to the 1.76.0 milestone Nov 12, 2023

fmease deleted the deny-more-tilde-const branch November 12, 2023 07:12

bors mentioned this pull request Nov 12, 2023

Avoid silencing relevant follow-up errors #117449

Merged

matthiaskrgr mentioned this pull request Nov 12, 2023

ICE: stack overflow #117831

Open

fee1-dead mentioned this pull request Dec 6, 2023

Tracking Issue for removing impl const and ~const in the standard library #110395

Open

4 tasks

fmease mentioned this pull request Dec 18, 2023

Deny ~const trait bounds in inherent impl headers #119059

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Deny more `~const` trait bounds #117817

Deny more `~const` trait bounds #117817

fmease commented Nov 11, 2023 •

edited

matthiaskrgr commented Nov 11, 2023

fee1-dead left a comment

fmease commented Nov 11, 2023

fmease Nov 12, 2023 •

edited

fee1-dead Nov 12, 2023

compiler-errors Dec 5, 2023

fee1-dead Nov 12, 2023 •

edited

fee1-dead commented Nov 12, 2023

bors commented Nov 12, 2023

bors commented Nov 12, 2023

bors commented Nov 12, 2023

rust-timer commented Nov 12, 2023

fmease commented Nov 12, 2023 •

edited

Deny more ~const trait bounds #117817

Deny more ~const trait bounds #117817

Conversation

fmease commented Nov 11, 2023 • edited

matthiaskrgr commented Nov 11, 2023

fee1-dead left a comment

Choose a reason for hiding this comment

fmease commented Nov 11, 2023

fmease Nov 12, 2023 • edited

Choose a reason for hiding this comment

fee1-dead Nov 12, 2023

Choose a reason for hiding this comment

compiler-errors Dec 5, 2023

Choose a reason for hiding this comment

fee1-dead Nov 12, 2023 • edited

Choose a reason for hiding this comment

fee1-dead commented Nov 12, 2023

bors commented Nov 12, 2023

bors commented Nov 12, 2023

bors commented Nov 12, 2023

rust-timer commented Nov 12, 2023

Overall result: ❌ regressions - no action needed

Instruction count

Max RSS (memory usage)

Cycles

Binary size

fmease commented Nov 12, 2023 • edited

Deny more `~const` trait bounds #117817

Deny more `~const` trait bounds #117817

fmease commented Nov 11, 2023 •

edited

fmease Nov 12, 2023 •

edited

fee1-dead Nov 12, 2023 •

edited

fmease commented Nov 12, 2023 •

edited