Skip to content

Conversation

@dimbleby
Copy link
Contributor

as discussed in #424

@highfive
Copy link

warning Warning warning

  • These commits modify unsafe code. Please review it carefully!

Copy link
Member

@fitzgen fitzgen left a comment

Choose a reason for hiding this comment

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

This looks great, thank you! I have just one nitpick below, and once that's fixed, let's merge this :)

// opportunity to generate tests for their layout here.

let layout = item.kind().expect_type().layout(ctx);
if ctx.options().layout_tests {
Copy link
Member

Choose a reason for hiding this comment

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

Rather than indenting tons of code, lets just early return here if we aren't generating layout tests.

concat!("Alignment of ", stringify!($type_name)));
)
};
if ctx.options().layout_tests {
Copy link
Member

Choose a reason for hiding this comment

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

Unfortunately, we can't early return in this one because there is more work happening afterwards. It would be awesome if this layout test generation was factored into its own function, but I realize that is an extant problem that is a bit out of scope for this PR.


if self.options.derive_debug == false {
if !self.options.layout_tests {
output_vector.push("--no-layout-tests".into());
Copy link
Member

Choose a reason for hiding this comment

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

Thanks for updating this method too!

@fitzgen
Copy link
Member

fitzgen commented Apr 14, 2017

@bors-servo r+

Thanks @dimbleby!

@bors-servo
Copy link

📌 Commit b8dd5c9 has been approved by fitzgen

@bors-servo
Copy link

⌛ Testing commit b8dd5c9 with merge 8b1f7c0...

bors-servo pushed a commit that referenced this pull request Apr 14, 2017
Option to avoid generating layout tests

as discussed in #424
@bors-servo
Copy link

☀️ Test successful - status-travis
Approved by: fitzgen
Pushing 8b1f7c0 to master...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants