Skip to content

ir: We really need to traverse all edges for the used template parameter analysis to be sound. #835

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Jul 21, 2017
Merged

ir: We really need to traverse all edges for the used template parameter analysis to be sound. #835

merged 2 commits into from
Jul 21, 2017

Conversation

emilio
Copy link
Contributor

@emilio emilio commented Jul 21, 2017

Fixes #833

@emilio
Copy link
Contributor Author

emilio commented Jul 21, 2017

This "regresses" #834 to the original behavior. If we really want to do that, we need to split the whitelisted items analysis from the "codegen'd types".

r? @fitzgen

This was referenced Jul 21, 2017
Closed
Merged
@fitzgen
Copy link
Member

fitzgen commented Jul 21, 2017

Oh, I see you already got this :-P

@fitzgen
Copy link
Member

fitzgen commented Jul 21, 2017

I wonder if its worth having both codegen_items and whitelist_items for the set of things that are whitelisted and we're codegen'ing and for the set of whitelisted regardless of codegen respectively...

@bors-servo r+

@bors-servo
Copy link

📌 Commit ba9298d has been approved by fitzgen

@bors-servo
Copy link

⌛ Testing commit ba9298d with merge 1830c2d...

bors-servo pushed a commit that referenced this pull request Jul 21, 2017
Auto merge of #835 - emilio:tp-function, r=fitzgen
1830c2d 
 	ir: We really need to traverse all edges for the used template parameter analysis to be sound.

Fixes #833
@bors-servo
Copy link

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

@bors-servo bors-servo merged commit ba9298d into rust-lang:master Jul 21, 2017
@Manishearth
Copy link
Member

We seem to be still having issues with this, but only on linux64 (not max or linux32)

https://treeherder.mozilla.org/#/jobs?repo=try&revision=0e1c83df4f86f368bbe2c6bf8e6d2caef2959c89

@Manishearth
Copy link
Member

We seem to be still having issues with this, but only on linux

https://treeherder.mozilla.org/#/jobs?repo=try&revision=0e1c83df4f86f368bbe2c6bf8e6d2caef2959c89

@fitzgen fitzgen mentioned this pull request Jul 25, 2017
Closed
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@emilio @fitzgen @bors-servo @Manishearth @highfive