-
Notifications
You must be signed in to change notification settings - Fork 12.8k
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
Various cleanups of macro-expanding-to-module workarounds #10719
Conversation
@@ -173,8 +173,7 @@ impl CString { | |||
/// Return a CString iterator. | |||
pub fn iter<'a>(&'a self) -> CStringIterator<'a> { | |||
CStringIterator { | |||
ptr: self.buf, | |||
lifetime: unsafe { cast::transmute(self.buf) }, | |||
ptr: self.buf |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This isn't removing a macro work-around, right? Would it be possible to have the macro stuff and the #5922 stuff in separate commits?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
They both meet in the VecIterator
implementation macro. Separating them would mean introducing additional changes just to remove them again in the next commit.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably worth at least mentioning that you're removing some dummy fields that gives structs lifetimes in the PR text/commit message.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm, also, #10703 probably means it's too early to remove them now, as it will allow people to write invalid/unsafe/crashing code.
Also documented a few issues
@@ -8,7 +8,9 @@ | |||
// option. This file may not be copied, modified, or distributed | |||
// except according to those terms. | |||
|
|||
// FIXME(#4375): this shouldn't have to be a nested module named 'generated' | |||
// FIXME(#4375): This shouldn't have to be a nested module named 'generated'... | |||
// FIXME(#10716): ... but now that we could solve that, the import lines and |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should remove the previous FIXME because it's no longer an open issue. I would personally be ok with just copy-pasting all of the attributes/include statements in each of the files using this macro (it's a big step forward in having reasonable documentation for these modules)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, will do that then
…excrichton - Removed module reexport workaround for the integer module macros - Removed legacy reexports of `cmp::{min, max}` in the integer module macros - Combined a few macros in `vec` into one - Documented a few issues
…, r=jonas-schievink Restart proc-macro client when server reload Fix rust-lang#10719
…rted_modules, r=Alexendoo Fix `items_after_test_module`: Ignore imported modules Fixes rust-lang#10713. It does a little bit of dark magic, but intention is what really counts. changelog:[`items_after_test_module`]: Ignore imported modules (`mod foo;`) with no body.
cmp::{min, max}
in the integer module macrosvec
into one