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

Move `lib/support.rs` to zinc crate #119

Merged
merged 1 commit into from Aug 5, 2014

Conversation

Projects
None yet
3 participants
@errordeveloper
Member

errordeveloper commented Jul 29, 2014

As discussed in #106, this removes a bunch of functions that are no longer needed and only keeps the ones it doesn't compile with. It's been verified that there no symbols pulled into the ELFs from anywhere that would be the same as the ones removed by this change.

@errordeveloper errordeveloper changed the title from Move `lib/support.rs` to zinc crate to Move `lib/support.rs` to zinc crate (RE: #106) Jul 29, 2014

@errordeveloper errordeveloper changed the title from Move `lib/support.rs` to zinc crate (RE: #106) to Move `lib/support.rs` to zinc crate Jul 29, 2014

Rakefile Outdated
@@ -106,7 +96,7 @@ desc "Build API documentation"
task build_docs: [:build_docs_html]
task build_docs_html: [] do |t|
['src/main.rs', 'platformtree/platformtree.rs'].each do |f|
['src/lib.rs', 'platformtree/platformtree.rs'].each do |f|

This comment has been minimized.

@farcaller

farcaller Jul 29, 2014

Member

You might want to revert the name change back then.

src/main.rs Outdated
@@ -55,6 +55,45 @@ pub mod hal;
pub mod lib;
pub mod os;
//#![feature(asm, intrinsics)]

This comment has been minimized.

@farcaller

farcaller Jul 29, 2014

Member

Remove this.

src/main.rs Outdated
@@ -55,6 +55,45 @@ pub mod hal;
pub mod lib;
pub mod os;
//#![feature(asm, intrinsics)]
#[cfg(test)]

This comment has been minimized.

@farcaller

farcaller Jul 29, 2014

Member

FWIW, I think it was quite in place in src/lib/support.rs, it's just the separate create that was useless.

This comment has been minimized.

@errordeveloper

errordeveloper Jul 29, 2014

Member

That's what I was thinking at the start! Thanks, will move it back 👍

#![feature(asm, intrinsics)]
//! Support functions currently required by the linker for bare-metal targets.
#![allow(missing_doc)]

This comment has been minimized.

@bharrisau

bharrisau Jul 30, 2014

Contributor

Document abort and breakpoint. You can #allow the other two.

This comment has been minimized.

@errordeveloper

errordeveloper Jul 30, 2014

Member

Well, I thought those two very like most self-explanatory the anything else. I'll think of how to describe them, but suggestions are welcome 👍

This comment has been minimized.

@bharrisau

bharrisau Jul 30, 2014

Contributor

abort: Call debugger and halt execution.
breakpoint: Call debugger.

This comment has been minimized.

@bharrisau

bharrisau Jul 30, 2014

Contributor

Actually, can you #[doc(hidden)] those other ones. If it is only the compiler calling them, there is no need to appear in documentation.

src/main.rs Outdated
@@ -55,6 +55,8 @@ pub mod hal;
pub mod lib;
pub mod os;
#[path="lib/support.rs"] pub mod support;

This comment has been minimized.

@bharrisau

bharrisau Jul 30, 2014

Contributor

Why not include it in lib/mod.rs?

This comment has been minimized.

@errordeveloper

errordeveloper Jul 30, 2014

Member

I couldn't quite figure out how to include it otherwise... Any suggestions? Those symbols need to visible globally.

This comment has been minimized.

@bharrisau

bharrisau Jul 30, 2014

Contributor

And what I suggested above doesn't work??

This comment has been minimized.

@farcaller

farcaller Jul 30, 2014

Member

lib is public, and the names are not mangled anyway, pub mod support in lib/mod.rs should work.

@errordeveloper

This comment has been minimized.

Member

errordeveloper commented Jul 30, 2014

/// Call debugger.

This comment has been minimized.

@bharrisau

bharrisau Jul 30, 2014

Contributor

Just make this a doc hidden or doc_missing. It is only for test.

/// Call debugger.

This comment has been minimized.

@bharrisau

bharrisau Jul 30, 2014

Contributor

Can you change this to "Calls the" - in line with this discussion #121 (comment)

unsafe { asm!("bkpt") }
}
/// Call debugger and halt execution.

This comment has been minimized.

@bharrisau

bharrisau Jul 30, 2014

Contributor

Same thing here "Calls the debugger and halts execution."

@bharrisau

This comment has been minimized.

Contributor

bharrisau commented Jul 30, 2014

r=me with the changes above.

@errordeveloper

This comment has been minimized.

Member

errordeveloper commented Jul 31, 2014

r=bharrisau

@bharrisau

This comment has been minimized.

Contributor

bharrisau commented Jul 31, 2014

You need to write it on the commit.

@bharrisau

This comment has been minimized.

Contributor

bharrisau commented Jul 31, 2014

And you didn't rewrite the doc comments? "Calls the debugger and halts execution."

@errordeveloper

This comment has been minimized.

Member

errordeveloper commented Jul 31, 2014

And you didn't rewrite the doc comments? "Calls the debugger and halts execution."

Fixed.

Move `lib/support.rs` to zinc crate -
As discussed in #106, this removes a bunch of functions that are no
longer needed and only keeps the ones it doesn't compile with. It's been
verified that there no symbols pulled into the ELFs from anywhere that
would be the same as the ones removed by this change.
@bharrisau

This comment has been minimized.

Contributor

bharrisau commented Aug 1, 2014

Not sure why bors didn't pick this up. @farcaller ?

hacknbot added a commit that referenced this pull request Aug 1, 2014

Merge pull request #119 from errordeveloper/move_libsupport
Move `lib/support.rs` to zinc crate

Reviewed-by: bharrisau
@errordeveloper

This comment has been minimized.

Owner

errordeveloper commented on 54f8cc9 Aug 1, 2014

Anyhow, I just rebased this... Let's try again, r=bharrisau 👍

This comment has been minimized.

Owner

errordeveloper replied Aug 1, 2014

r=bharrisau

@farcaller

This comment has been minimized.

Member

farcaller commented Aug 1, 2014

It recently got stuck in rebase loop but should be fine now.

@farcaller

This comment has been minimized.

Member

farcaller commented on 54f8cc9 Aug 1, 2014

r=bharrisau

@errordeveloper

This comment has been minimized.

Member

errordeveloper commented Aug 5, 2014

@bharrisau @farcaller are we merging this?

@farcaller

This comment has been minimized.

Member

farcaller commented Aug 5, 2014

Sorry, I got a bit carried away with all the stuff.

farcaller added a commit that referenced this pull request Aug 5, 2014

Merge pull request #119 from errordeveloper/move_libsupport
Move `lib/support.rs` to zinc crate

@farcaller farcaller merged commit 80d5bac into hackndev:master Aug 5, 2014

1 check passed

continuous-integration/travis-ci The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment