Repository navigation
Conversation
|
Some changes occurred to the CTFE machinery The reflection data structures are tied exactly to the implementation cc @oli-obk Some changes occurred to the intrinsics. Make sure the CTFE / Miri interpreter |
This comment has been minimized.
This comment has been minimized.
a59d1d8 to
c33e84e
Compare
| )?; | ||
| } | ||
|
|
||
| sym::type_id_docs => { |
There was a problem hiding this comment.
The list of intrinsics here is growing so much that I think it'd make sense to move it to a separate file. (In a separate PR.)
This comment has been minimized.
This comment has been minimized.
c33e84e to
d834d5c
Compare
| } | ||
| } | ||
|
|
||
| sym::type_id_variant_docs => { |
There was a problem hiding this comment.
Maybe we should add a variant-pattern type (similar to what we do with FRTs instead of also having a field variant of many intrinsics) sooner rather than later instead of having to have all these intrinsics duplicated for variants and for normal types. Not for this PR, but just adding them to the type system without exposing them at the AST level shouldn't be too hard (famous last words)
|
Reminder, once the PR becomes ready for a review, use |
d834d5c to
b6997cd
Compare
…r=oli-obk reflection: doc attributes from adts, fields, and variants Adds intrinsics and methods to get the raw doc comment strings from adts, fields, and variants. As noted in the tracking issue rust-lang#146922, before stabilizing the interface we should consider whether these methods get the raw strings. r? @oli-obk
|
💔 I suspect this PR failed tests as part of a rollup After fixing the problem, consider running a try job for the failed job before re-approving. Link to failure: #165134 (comment) Soft conflict with #165079, please wait for that to merge then rebase |
|
This pull request was unapproved. This PR was contained in a rollup (#165134), which was unapproved. |
|
Can you also update the comment here (assuming this is intended to work cross crate) rust/compiler/rustc_metadata/src/rmeta/encoder.rs Lines 952 to 958 in 2687a50 |
b6997cd to
f7f8bbc
Compare
|
Thanks for the suggestions, incorporated them. Regarding the soundness of having doc comments in the type system, I think @yara-blue can provide more insight in it. @rustbot ready |
Are you concerned about semver? It is a wider concern within reflection we have some solutions in mind, ill expand on those later once they are worked out a bit more. |
|
Sorry for being a bit vague earlier, I didn't have the time to properly investigate and come up with an example. We do not always keep doc comments around (see To illustrate, the following snippet has undefined behavior: // crate `dep`
#![feature(type_info)]
use std::any::TypeId;
pub const N: usize = {
struct TestStruct {
/// 🦀
field: u8,
}
let ty_id = TypeId::of::<TestStruct>();
ty_id.field(0, 0).docs().len()
};
pub fn get_n() -> usize {
N
}
pub fn unpack(x: [String; N]) -> String {
let [s] = x;
s
}fn main(){
println!("{}, {}", dep::N, dep::get_n()); // 0, 1
let x = [String::new(); dep::N];
let x = dep::unpack(x);
println!("{}", x);
} |
|
Think that makes sense. Will take look when I have more time later next week. @rustbot author |
ahh I get it, thats a problem indeed. Will run into the same with other attributes as well. Thanks for that clear example! We should adopt it as a test. |
|
I think just always encoding doc comments is the easiest fix, if perf allows it (perf run at #165304). Alternatives include maybe some way to mark things as "effectively visible" if their type info is used. Or maybe |
|
☔ The latest upstream changes (presumably #165350) made this pull request unmergeable. Please resolve the merge conflicts by rebasing. |
Adds intrinsics and methods to get the raw doc comment strings from adts, fields, and variants. As noted in the tracking issue #146922, before stabilizing the interface we should consider whether these methods get the raw strings.
r? @oli-obk