Skip to content

rustc_trans: apply the debug location for the MIR Assert panic call. - #35141

Merged
bors merged 1 commit into
rust-lang:masterfrom
eddyb:assert-mir-debug
Aug 1, 2016
Merged

rustc_trans: apply the debug location for the MIR Assert panic call.#35141
bors merged 1 commit into
rust-lang:masterfrom
eddyb:assert-mir-debug

Conversation

@eddyb

@eddybeddyb commented Jul 31, 2016

Copy link
Copy Markdown
Contributor

Helps libcore build with MIR trans and debuginfo; libcore has the body of panic, which resulted in:

inlinable function call in a function with debug info must have a !dbg location
call void @_ZN4core9panicking5panic17h585bd70cda921012E({ %str_slice, %str_slice, i32 }* @panic_loc12745)
LLVM ERROR: Broken function found, compilation aborted!

@rust-highfive

rust-highfive commented Jul 31, 2016

Copy link
Copy Markdown
Contributor

r? @nikomatsakis

(rust_highfive has picked a reviewer for you, use r? to override)

@eddybeddyb mentioned this pull request Jul 31, 2016
Closed
nagisa
nagisa reviewed Jul 31, 2016
View reviewed changes
Comment threadsrc/librustc_trans/mir/block.rs

// After this point, bcx is the block for the call to panic.
bcx = panic_block.build();
debug_loc.apply_to_bcx(&bcx);

@nagisanagisaJul 31, 2016

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I feel like this is a fairly easy stuff to overlook. I skimmed the context and haven’t found any more obvious instances of such mistake, but I feel like it is considerably easy to make similar mistake at any point in the future.

Perhaps we should consider making a wrapper of some sort for fcx.new_block?

@eddybeddybJul 31, 2016

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

When old trans is gone, we can have a simpler and less error-prone interface, i.e. a Block that always knows its debug position.

@nagisa

nagisa commented Jul 31, 2016

Copy link
Copy Markdown
Member

r=me, but I have a feeling that I will see a similar PR at some time in the future again.

@eddyb

eddyb commented Jul 31, 2016

Copy link
Copy Markdown
ContributorAuthor

@bors r=nagisa rollup

@bors

bors commented Jul 31, 2016

Copy link
Copy Markdown
Collaborator

📌 Commit d1f341d has been approved by nagisa

sanxiyn added a commit to sanxiyn/rust that referenced this pull request Aug 1, 2016
@sanxiyn
Rollup merge ofrust-lang#35141- eddyb:assert-mir-debug, r=nagisa
dc63b3a
rustc_trans: apply the debug location for the MIR Assert panic call.
Helps `libcore` build with MIR trans and debuginfo; libcore has the body of `panic`, which resulted in:
```
inlinable function call in a function with debug info must have a !dbg location
call void @_ZN4core9panicking5panic17h585bd70cda921012E({ %str_slice, %str_slice, i32 }* @panic_loc12745)
LLVM ERROR: Broken function found, compilation aborted!
```
@sanxiynsanxiyn mentioned this pull request Aug 1, 2016
Merged
bors added a commit that referenced this pull request Aug 1, 2016
@bors
Auto merge of#35163- sanxiyn:rollup, r=sanxiyn
28ce3e8
Rollup of 8 pull requests
- Successful merges: #34802, #35033, #35085, #35114, #35134, #35140, #35141, #35157
- Failed merges:
@bors
bors merged commit d1f341d into rust-lang:masterAug 1, 2016
@petrochenkovpetrochenkov mentioned this pull request Aug 10, 2016
Closed
@eddyb
eddyb deleted the assert-mir-debug branch March 9, 2017 13:40
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@nikomatsakisnikomatsakis

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@eddyb@rust-highfive@nagisa@bors@nikomatsakis