Skip to content
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

rustc: remove dead code #13244

Merged
merged 45 commits into from
Apr 3, 2014
Merged

rustc: remove dead code #13244

merged 45 commits into from
Apr 3, 2014

Conversation

emberian
Copy link
Member

@emberian emberian commented Apr 1, 2014

No description provided.

pub static box_field_refcnt: uint = 0u;
pub static box_field_tydesc: uint = 1u;
pub static box_field_prev: uint = 2u;
pub static box_field_next: uint = 3u;
pub static box_field_body: uint = 4u;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be made 2u ? (same question for other cases)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is used for the offset of the field in the struct. this is hard-coded, and doesn't change with order. if it works today, it will continue to work.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I know it'll continue to work. It was more a "Do we want this to be sequential?"

Either way works for me, TBH.

@emberian
Copy link
Member Author

emberian commented Apr 1, 2014

@flaper87 pushed new version

@@ -1920,10 +1863,6 @@ def_type_content_sets!(
)

impl TypeContents {
pub fn meets_bounds(&self, cx: &ctxt, bbs: BuiltinBounds) -> bool {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is useful. I'm actually surprised that middle::kinds::check_builtin_bounds doesn't use this.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, this will probably go away with OIBT, so, feel free to add a FIXME(#13231)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not used today, and if it's going to go away.... why keep it?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was more a "I'm surprised it's not being used, I think I'll need it but I'm not sure yet" :D Anyway, ignore this comment. Lets get rid of it.

@flaper87
Copy link
Contributor

flaper87 commented Apr 1, 2014

Added a few more comments. Sorry for not catching those before.

pub static tag_items_data_item_visibility: uint = 0x4e;

pub static tag_link_args: uint = 0x4f;
pub static tag_link_args_arg: uint = 0x50;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same concern in this area: maybe numbers could be reassigned sequentially? (maybe a macro could do this automatically?)

@emberian
Copy link
Member Author

emberian commented Apr 2, 2014

review addressed

bors added a commit that referenced this pull request Apr 3, 2014
@bors bors closed this Apr 3, 2014
@bors bors merged commit 46790a7 into rust-lang:master Apr 3, 2014
@lilyball
Copy link
Contributor

lilyball commented Apr 4, 2014

I was very confused when reading the git history, because of the name of this branch.

@emberian
Copy link
Member Author

emberian commented Apr 4, 2014

sorry :) I noticed that the surrounding area needed some cleanup as I was experimenting

@flaper87
Copy link
Contributor

flaper87 commented Apr 4, 2014

yay! 🍰

arcnmx pushed a commit to arcnmx/rust that referenced this pull request Dec 17, 2022
…r=Veykril

Support builtin derive macro helper attributes

Closes rust-lang#13244

It's a bit wasteful for `Macro2Data` to have `helpers` field currently just for `Default` derive macro, but I tend to think it's okay for the time being given how rare macro2's are used.
Jarcho pushed a commit to Jarcho/rust that referenced this pull request Aug 24, 2024
Remove more `snippet_opt` calls

First commit is the same as rust-lang#13244

changelog: none
Jarcho pushed a commit to Jarcho/rust that referenced this pull request Aug 24, 2024
Start removing `snippet_opt` in favor of `get_source_text`

Continuing the job of removing unnecessary allocations.

changelog: none
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants