Effects/keyword generics MVP #113210

fee1-dead · 2023-06-30T16:18:13Z

This adds feature(effects), which adds const host: bool to the generics of const functions, const traits and const impls. This will be used to replace the current logic around const traits.

r? @oli-obk

Noratrieb · 2023-06-30T17:01:20Z

compiler/rustc_feature/src/builtin_attrs.rs

@@ -723,6 +723,12 @@ pub const BUILTIN_ATTRIBUTES: &[BuiltinAttribute] = &[
        and it is only intended to be used in `alloc`."
    ),

+    rustc_attr!(
+        rustc_host, AttributeType::Normal, template!(Word), ErrorFollowing,


I find the name host really confusing here, as I've been told that HOST=true means not-const, which goes directly against my intuition of HOST=true` meaning const, because the code is executed on the host compiler.

oli-obk · 2023-07-04T10:45:00Z

compiler/rustc_ast_lowering/src/item.rs

+                let constness = if let Some(attrs) = attrs {
+                    attrs
+                        .iter()
+                        .find(|x| x.has_name(sym::const_trait))
+                        .map_or(Const::No, |x| Const::Yes(x.span))
+                } else {
+                    Const::No
+                };


let constness = attrs .iter() .flatten() .find(|x| x.has_name(sym::const_trait)) .map_or(Const::No, |x| Const::Yes(x.span))

oli-obk · 2023-07-04T10:49:59Z

compiler/rustc_hir_typeck/src/callee.rs

+
+        // FIXME this should be made more efficient
+        let host_effect_param_index = identity_substs.iter().position(|x| {
+            matches!(x.unpack(), ty::GenericArgKind::Const(const_) if matches!(const_.kind(), ty::ConstKind::Param(param) if param.name == sym::host))


This should probably stop looking at the parameter name at some point and check whether it has a rustc_host attribute

oli-obk · 2023-07-04T10:50:29Z

compiler/rustc_hir_typeck/src/callee.rs

+
+        trace!(?effect, ?identity_substs, ?callee_substs);
+
+        // FIXME this should be made more efficient


I think you could cache the index in a field in the ty::Generics by precomputing it once in the generics_of query. You should use the generics_of query instead of identity substs. Though that will require some index shenanigans to find the right index (need to add to the parent count).

compiler/rustc_hir_typeck/src/callee.rs

oli-obk

r=me on the PR with the constness nit unless you also wanna do the rename, that Nils asked for, in this PR

oli-obk · 2023-07-04T17:02:32Z

compiler/rustc_middle/src/ty/generics.rs

@@ -133,6 +133,9 @@ pub struct Generics {

    pub has_self: bool,
    pub has_late_bound_regions: Option<Span>,
+
+    // The index of the constness effect when substituted. (i.e. might be index to parent substs)


s/constness/host

fee1-dead · 2023-07-04T17:24:39Z

Will rename in a followup.

@bors r=oli-obk

bors · 2023-07-04T17:24:41Z

📌 Commit 25fc6c1 has been approved by oli-obk

It is now in the queue for this repository.

…li-obk Effects/keyword generics MVP This adds `feature(effects)`, which adds `const host: bool` to the generics of const functions, const traits and const impls. This will be used to replace the current logic around const traits. r? `@oli-obk`

bors · 2023-07-05T13:42:03Z

⌛ Testing commit 25fc6c1 with merge e4cd161...

bors · 2023-07-05T16:08:36Z

☀️ Test successful - checks-actions
Approved by: oli-obk
Pushing e4cd161 to master...

rust-timer · 2023-07-05T17:24:52Z

Finished benchmarking commit (e4cd161): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

Instruction count

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

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.5%	[1.8%, 3.8%]	7
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-3.6%	[-5.1%, -2.7%]	3
All ❌✅ (primary)	-	-	0

Cycles

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

Binary size

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.1%	[0.0%, 0.3%]	91
Regressions ❌ (secondary)	0.2%	[0.0%, 0.4%]	46
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	0.1%	[0.0%, 0.3%]	91

Bootstrap: 653.976s -> 655.622s (0.25%)

rustbot assigned oli-obk Jun 30, 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 Jun 30, 2023

This comment has been minimized.

Sign in to view

fee1-dead force-pushed the effects-mvp branch from 76ff3cf to d80496a Compare June 30, 2023 16:23

Noratrieb reviewed Jun 30, 2023

View reviewed changes

oli-obk requested changes Jul 4, 2023

View reviewed changes

fee1-dead added 5 commits July 4, 2023 11:47

Add feature and attribute definition

2d5d56b

Add effects during lowering for ~const bounds

1c837cb

enforce context effects in typeck

8eb3e0b

allow host param to be lowercase

0b095a6

add test

30b21b7

fee1-dead force-pushed the effects-mvp branch from d80496a to 32df51b Compare July 4, 2023 13:51

oli-obk requested changes Jul 4, 2023

View reviewed changes

fee1-dead added 2 commits July 4, 2023 17:21

include host_effect_index in Generics

58c105a

add fixme

25fc6c1

fee1-dead force-pushed the effects-mvp branch from 21ddd97 to 25fc6c1 Compare July 4, 2023 17:23

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 4, 2023

Dylan-DPC mentioned this pull request Jul 5, 2023

Rollup of 7 pull requests #113357

Closed

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

bors merged commit e4cd161 into rust-lang:master Jul 5, 2023
12 checks passed

rustbot added this to the 1.72.0 milestone Jul 5, 2023

matthiaskrgr mentioned this pull request Jul 5, 2023

ICE: effects: index out of bounds compiler/rustc_hir_typeck/src/callee.rs #113375

Closed

fee1-dead deleted the effects-mvp branch July 6, 2023 01:16

fee1-dead mentioned this pull request May 21, 2023

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

Open

4 tasks

matthiaskrgr mentioned this pull request Nov 27, 2023

ice: ConstContext::Maybe must have host effect param #118320

Open

matthiaskrgr mentioned this pull request Jan 17, 2024

ice: assertion failed: constant / lifetime with effects and try_trait #119717

Closed

lqd mentioned this pull request May 30, 2024

ICE: !self.intercrate #125767

Closed

celinval mentioned this pull request Jun 12, 2024

Cannot verify a constant function if crate has effect feature enabled model-checking/kani#3258

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Effects/keyword generics MVP #113210

Effects/keyword generics MVP #113210

fee1-dead commented Jun 30, 2023

This comment has been minimized.

Noratrieb Jun 30, 2023

oli-obk Jul 4, 2023

oli-obk Jul 4, 2023

oli-obk Jul 4, 2023

oli-obk left a comment •

edited

Loading

oli-obk Jul 4, 2023

fee1-dead commented Jul 4, 2023

bors commented Jul 4, 2023

bors commented Jul 5, 2023

bors commented Jul 5, 2023

rust-timer commented Jul 5, 2023


		trace!(?effect, ?identity_substs, ?callee_substs);

		// FIXME this should be made more efficient

Effects/keyword generics MVP #113210

Effects/keyword generics MVP #113210

Conversation

fee1-dead commented Jun 30, 2023

This comment has been minimized.

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

oli-obk left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fee1-dead commented Jul 4, 2023

bors commented Jul 4, 2023

bors commented Jul 5, 2023

bors commented Jul 5, 2023

rust-timer commented Jul 5, 2023

Overall result: no relevant changes - no action needed

oli-obk left a comment •

edited

Loading