-
Notifications
You must be signed in to change notification settings - Fork 12.7k
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
Effects/keyword generics MVP #113210
Effects/keyword generics MVP #113210
Conversation
This comment has been minimized.
This comment has been minimized.
@@ -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, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
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 | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
let constness = attrs
.iter()
.flatten()
.find(|x| x.has_name(sym::const_trait))
.map_or(Const::No, |x| Const::Yes(x.span))
|
||
// 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)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should probably stop looking at the parameter name at some point and check whether it has a rustc_host
attribute
|
||
trace!(?effect, ?identity_substs, ?callee_substs); | ||
|
||
// FIXME this should be made more efficient |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
r=me on the PR with the constness nit unless you also wanna do the rename, that Nils asked for, in this PR
@@ -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) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
s/constness/host
Will rename in a followup. @bors r=oli-obk |
…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`
☀️ Test successful - checks-actions |
Finished benchmarking commit (e4cd161): comparison URL. Overall result: no relevant changes - no action needed@rustbot label: -perf-regression Instruction countThis benchmark run did not return any relevant results for this metric. Max RSS (memory usage)ResultsThis 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.
CyclesThis benchmark run did not return any relevant results for this metric. Binary sizeResultsThis 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.
Bootstrap: 653.976s -> 655.622s (0.25%) |
This adds
feature(effects)
, which addsconst 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