Skip to content
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

Make bitfield unit allocation fallible #1141

Merged
merged 1 commit into from
Nov 3, 2017

Conversation

fitzgen
Copy link
Member

@fitzgen fitzgen commented Nov 2, 2017

Instead of panicking when we see a bitfield that does not have a layout, return an error up the stack. If we get an error when allocating bitfields into units, then make the whole struct opaque.

Fixes #1140

r? @pepyakin or @emilio

Copy link
Contributor

@emilio emilio left a comment

Choose a reason for hiding this comment

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

r=me, thanks for the fix! :)

@@ -466,7 +466,7 @@ impl FieldMethods for RawField {
fn raw_fields_to_fields_and_bitfield_units<I>(
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you add to the doc comment when do we return errors?

src/ir/comp.rs Outdated
@@ -1046,6 +1065,7 @@ impl CompInfo {
/// Get this type's set of fields.
pub fn fields(&self) -> &[Field] {
match self.fields {
CompFields::ErrorComputingBitfieldUnits => &[][..],
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: I think the [..] is not needed.

src/ir/comp.rs Outdated
@@ -1558,6 +1578,10 @@ impl IsOpaque for CompInfo {
return true
}

if let CompFields::ErrorComputingBitfieldUnits = self.fields {
Copy link
Contributor

Choose a reason for hiding this comment

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

Note when can this happen? (here or somewhere else where it may make sense)

Instead of panicking when we see a bitfield that does not have a layout, return
an error up the stack. If we get an error when allocating bitfields into units,
then make the whole struct opaque.

Fixes rust-lang#1140
@fitzgen
Copy link
Member Author

fitzgen commented Nov 3, 2017

Thanks for the review! Good calls on the comments, I've updated the commit.

@bors-servo r+

@bors-servo
Copy link

📌 Commit 43b88c5 has been approved by fitzgen

@bors-servo
Copy link

⌛ Testing commit 43b88c5 with merge 40b083e...

bors-servo pushed a commit that referenced this pull request Nov 3, 2017
Make bitfield unit allocation fallible

Instead of panicking when we see a bitfield that does not have a layout, return an error up the stack. If we get an error when allocating bitfields into units, then make the whole struct opaque.

Fixes #1140

r? @pepyakin or @emilio
@fitzgen
Copy link
Member Author

fitzgen commented Nov 3, 2017

@bors-servo r-

@fitzgen
Copy link
Member Author

fitzgen commented Nov 3, 2017

@bors-servo r=emilio

d'oh

@bors-servo
Copy link

📌 Commit 43b88c5 has been approved by emilio

@highfive highfive assigned emilio and unassigned fitzgen Nov 3, 2017
@bors-servo
Copy link

⌛ Testing commit 43b88c5 with merge eccc0bf...

bors-servo pushed a commit that referenced this pull request Nov 3, 2017
Make bitfield unit allocation fallible

Instead of panicking when we see a bitfield that does not have a layout, return an error up the stack. If we get an error when allocating bitfields into units, then make the whole struct opaque.

Fixes #1140

r? @pepyakin or @emilio
@bors-servo
Copy link

☀️ Test successful - status-travis
Approved by: emilio
Pushing eccc0bf to master...

@bors-servo bors-servo merged commit 43b88c5 into rust-lang:master Nov 3, 2017
@fitzgen fitzgen deleted the bitfield-without-layout branch November 3, 2017 16:43
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.

None yet

5 participants