Skip to content

codegen: Reuse the next_child_local_id hack for template instantiations. #708

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

Merged
merged 1 commit into from
May 20, 2017

Conversation

emilio
Copy link
Contributor

@emilio emilio commented May 19, 2017

This should be good enough, following the pattern of anonymous items, and should
prevent most of the current noise in stylo updates.

Closes #620
Fixes #619

@emilio
Copy link
Contributor Author

emilio commented May 19, 2017

r? @fitzgen

This should be good enough, following the pattern of anonymous items, and should
prevent most of the current noise in stylo updates.

Closes rust-lang#620
Fixes rust-lang#619
@emilio emilio force-pushed the template-inst-noise branch from cc45375 to 44f6858 Compare May 20, 2017 01:32
@emilio
Copy link
Contributor Author

emilio commented May 20, 2017

This works, but uses the parent item which is not great. I could use the template definition, which would need resolving through typerefs and all that. That'd produce better results I think, but it's not clear if it's worth. Whatever you prefer, @fitzgen

Copy link
Member

@fitzgen fitzgen left a comment

Choose a reason for hiding this comment

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

Nice!

@fitzgen
Copy link
Member

fitzgen commented May 20, 2017

@bors-servo r+

@bors-servo
Copy link

📌 Commit 44f6858 has been approved by fitzgen

@bors-servo
Copy link

⌛ Testing commit 44f6858 with merge ea9a932...

bors-servo pushed a commit that referenced this pull request May 20, 2017
codegen: Reuse the next_child_local_id hack for template instantiations.

This should be good enough, following the pattern of anonymous items, and should
prevent most of the current noise in stylo updates.

Closes #620
Fixes #619
@bors-servo
Copy link

☀️ Test successful - status-travis
Approved by: fitzgen
Pushing ea9a932 to master...

@bors-servo bors-servo merged commit 44f6858 into rust-lang:master May 20, 2017
@emilio emilio deleted the template-inst-noise branch May 20, 2017 08:52
@upsuper
Copy link
Contributor

upsuper commented May 27, 2017

This is still pretty painful, because some template types are used widely (e.g. nsTArray, UniquePtr), and there can still be tons of different instantiations. I guess it is still better to have mangled name there rather than id.

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