Skip to content

Remove unused LLVM related code #47233

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Jump to bottom
Merged
merged 4 commits into from
Jan 9, 2018
Merged

Remove unused LLVM related code #47233

merged 4 commits into from
Jan 9, 2018

Conversation

dotdash
Copy link
Contributor

@dotdash dotdash commented Jan 6, 2018

Ticks a few more boxes on #46437

@rust-highfive
Copy link
Contributor

r? @arielb1

(rust_highfive has picked a reviewer for you, use r? to override)

hanna-kruppe
hanna-kruppe reviewed Jan 6, 2018
View reviewed changes
Copy link
Contributor

@hanna-kruppe hanna-kruppe left a comment

Choose a reason for hiding this comment

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

LGTM except removing the -Z flag.

src/librustc/session/config.rs Outdated
@@ -1082,8 +1082,6 @@ options! {DebuggingOptions, DebuggingSetter, basic_debugging_options,
"gather borrowck statistics"),
no_landing_pads: bool = (false, parse_bool, [TRACKED],
"omit landing pads for unwinding"),
debug_llvm: bool = (false, parse_bool, [UNTRACKED],
Copy link
Contributor

Choose a reason for hiding this comment

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

Is the idea behind removing this flag that one can pass -Cllvm-args=-debug instead? I'm fine with that, but it's not exactly "unused".

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh yes, now I see that the commit message mentions precisely that. As I said I'm fine with it but it's kind of a visible change so I don't feel comfortable giving r+ myself.

@alexcrichton
Copy link
Member

@bors: r+

Nice!

@bors
Copy link
Collaborator

bors commented Jan 6, 2018

📌 Commit 0b00ee7 has been approved by alexcrichton

@kennytm kennytm mentioned this pull request Jan 6, 2018
Merged
bors added a commit that referenced this pull request Jan 6, 2018
@bors
Auto merge of #47235 - kennytm:rollup, r=kennytm
6828cf9 
Rollup of 7 pull requests

- Successful merges: #46947, #47170, #47190, #47205, #47217, #47220, #47230
- Failed merges: #47233
@bors
Copy link
Collaborator

bors commented Jan 6, 2018

☔ The latest upstream changes (presumably #47235) made this pull request unmergeable. Please resolve the merge conflicts.

@dotdash
Copy link
Contributor Author

dotdash commented Jan 7, 2018

@bors r=alexcrichton

@bors
Copy link
Collaborator

bors commented Jan 7, 2018

📌 Commit 907855f has been approved by alexcrichton

@bors
Copy link
Collaborator

bors commented Jan 8, 2018

⌛ Testing commit 907855f with merge 30105f8d4ffe1f55e3a9a896abb576cc3e3f3625...

@bors
Copy link
Collaborator

bors commented Jan 8, 2018

💔 Test failed - status-appveyor

@dotdash
Copy link
Contributor Author

dotdash commented Jan 8, 2018

Looks like a timeout? I don't see any errors and the log seems to be cut off

@alexcrichton
Copy link
Member

@bors: retry

kennytm added a commit to kennytm/rust that referenced this pull request Jan 8, 2018
@kennytm
Rollup merge of rust-lang#47233 - dotdash:cleanup_llvm, r=alexcrichton
1cbbbc0 
Remove unused LLVM related code

Ticks a few more boxes on rust-lang#46437
@kennytm kennytm mentioned this pull request Jan 8, 2018
Merged
bors added a commit that referenced this pull request Jan 9, 2018
@bors
Auto merge of #47276 - kennytm:rollup, r=kennytm
74966b5 
Rollup of 10 pull requests

- Successful merges: #47210, #47233, #47246, #47254, #47256, #47258, #47259, #47263, #47270, #47272
- Failed merges: #47248
@bors bors merged commit 907855f into rust-lang:master Jan 9, 2018
@dotdash dotdash deleted the cleanup_llvm branch January 31, 2018 11:34
# for free to join this conversation on GitHub. Already have an account? # to comment
Reviewers

@hanna-kruppe hanna-kruppe hanna-kruppe left review comments

Assignees

@arielb1 arielb1

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@dotdash @rust-highfive @alexcrichton @bors @hanna-kruppe @arielb1