-
Notifications
You must be signed in to change notification settings - Fork 13.1k
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
Add a new attr indicates pub struct with private fields could be contructed externally #126456
Conversation
rustbot has assigned @compiler-errors. Use |
#126169 (comment) notes #126289. Please add a test and/or support for that. |
@compiler-errors updated the test |
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.
Also, this doesn't follow the usual process for adding new attributes to the compiler.
This either needs to be FCP'd by T-lang as an insta-stable attribute, or it needs to be feature gated appropriately.
@rustbot author Please add feature gating. I also wonder if we should bikeshed this a bit first before adding it, like on i-rl-o. |
This comment has been minimized.
This comment has been minimized.
@rustbot ready |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
@compiler-errors Sorry for the delay, I opened one on i-rl-o ;). link: https://internals.rust-lang.org/t/externally-constructed-or-anything-else/21049 |
compiler/rustc_passes/src/dead.rs
Outdated
} | ||
field.vis.is_public() | ||
}) | ||
tcx.has_attr(id, sym::externally_constructed) |
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.
Is this a good place for this check? In function check_item
, around line 838 you have a variable may_construct_self
and in visit_node
there is unconditionally_treats_fields_as_live
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.
Oh, seems checking it in create_and_seed_worklist
is good enough. And I opened another PR which just checks the repr(c)
or repr(transparent)
, see #127104.
…lds could be constructed externally
I believe the other approach to this is #127104? If so, I'm gonna close this. |
Fixes #126169