Skip to content

Conversation

ljedrz
Copy link
Contributor

@ljedrz ljedrz commented Oct 9, 2018

  • improve common patterns
  • convert string literals with to_owned
  • remove explicit returns
  • whitespace & formatting improvements

@ljedrz
Copy link
Contributor Author

ljedrz commented Oct 9, 2018

r? @michaelwoerister

@TimNN TimNN added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Oct 9, 2018
@ljedrz
Copy link
Contributor Author

ljedrz commented Oct 16, 2018

It seems @michaelwoerister might currently be unavailable; r? @varkor

Copy link
Contributor

@varkor varkor left a comment

Choose a reason for hiding this comment

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

Looks good to me apart from the semicolon nits.

None => { /* generate one */}
};
if let Some(unique_type_id) = self.type_to_unique_id.get(&type_).cloned() {
return unique_type_id
Copy link
Contributor

Choose a reason for hiding this comment

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

return as part of a statement should end with a semicolon. (Here and in the other changes.)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I wasn't aware of that; thanks, I'll update the PR shortly.

},
_ => { /* safe to ignore */ }
if let FunctionDebugContext::RegularContext(ref data) = *dbg_context {
data.source_locations_enabled.set(true)
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here.

@ljedrz
Copy link
Contributor Author

ljedrz commented Oct 16, 2018

@varkor thanks for the review! Semicolons added.

@varkor
Copy link
Contributor

varkor commented Oct 16, 2018

@bors r+ rollup

@bors
Copy link
Collaborator

bors commented Oct 16, 2018

📌 Commit 24b74b2 has been approved by varkor

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 16, 2018
kennytm added a commit to kennytm/rust that referenced this pull request Oct 18, 2018
…=varkor

Cleanup the rest of codegen_llvm

- improve common patterns
- convert string literals with `to_owned`
- remove explicit `return`s
- whitespace & formatting improvements
bors added a commit that referenced this pull request Oct 18, 2018
Rollup of 18 pull requests

Successful merges:

 - #54646 (improve documentation on std::thread::sleep)
 - #54933 (Cleanup the rest of codegen_llvm)
 - #54964 (Run both lldb and gdb tests)
 - #55016 (Deduplicate some code and compile-time values around vtables)
 - #55031 (Improve verify_llvm_ir config option)
 - #55050 (doc std::fmt: the Python inspiration is already mentioned in precedin…)
 - #55077 (rustdoc: Use dyn keyword when rendering dynamic traits)
 - #55080 (Detect if access to localStorage is forbidden by the user's browser)
 - #55090 (regression test for move out of borrow via pattern)
 - #55102 (resolve: Do not skip extern prelude during speculative resolution)
 - #55104 (Add test for #34229)
 - #55111 ([Rustc Book] Explain --cfg's arguments)
 - #55122 (Cleanup mir/borrowck)
 - #55127 (Remove HybridBitSet::dummy)
 - #55128 (Fix LLVMRustInlineAsmVerify return type mismatch)
 - #55142 (miri: layout should not affect CTFE checks (outside of validation))
 - #55151 (Cleanup nll)
 - #55161 ([librustdoc] Disable spellcheck for search field)
@bors bors merged commit 24b74b2 into rust-lang:master Oct 18, 2018
@ljedrz ljedrz deleted the cleanup_codegen_llvm/misc branch October 18, 2018 10:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants