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

Emit 1-based column numbers in debuginfo #69357

Merged
merged 3 commits into from Mar 15, 2020
Merged

Conversation

@tmiasko
Copy link

tmiasko commented Feb 21, 2020

  • Use byte offsets instead of char position offsets. Resolves #67360.
  • Use 1-based offsets instead of 0-based ones. Resolves #65437.
  • Consistently omit column information for msvc targets, matching clang behaviour (previously columns have been omitted from DILocation, but not from DILexicalBlock).
@rust-highfive

This comment has been minimized.

Copy link
Collaborator

rust-highfive commented Feb 21, 2020

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @matthewjasper (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

@ecstatic-morse

This comment has been minimized.

Copy link
Contributor

ecstatic-morse commented Feb 21, 2020

@tmiasko

This comment has been minimized.

Copy link
Author

tmiasko commented Feb 24, 2020

r? @eddyb

@rust-highfive rust-highfive assigned eddyb and unassigned matthewjasper Feb 24, 2020
@eddyb

This comment has been minimized.

Copy link
Member

eddyb commented Feb 24, 2020

cc @michaelwoerister I can't easily follow most of the original discussion.

Tomasz Miąsko added 3 commits Feb 21, 2020
The debuginfo column numbers are 1-based. The value 0 indicates that no
column has been specified. Translate 0-based column numbers to 1-based
when emitting debug information.
@tmiasko tmiasko force-pushed the tmiasko:debuginfo-column branch 2 times, most recently from 531e133 to 8e93a01 Feb 26, 2020
@tmiasko

This comment has been minimized.

Copy link
Author

tmiasko commented Feb 26, 2020

Updated to use byte position offsets instead of char positions offsets and address #67360.

@tmiasko

This comment has been minimized.

Copy link
Author

tmiasko commented Mar 10, 2020

@michaelwoerister

This comment has been minimized.

Copy link
Contributor

michaelwoerister commented Mar 11, 2020

Thanks, @tmiasko! Looks good to me.

@bors r+

@bors

This comment has been minimized.

Copy link
Contributor

bors commented Mar 11, 2020

📌 Commit 8e93a01 has been approved by michaelwoerister

pub fn create_debug_loc(&self, scope: &'ll DIScope, span: Span) -> &'ll Value {
let loc = span_start(self, span);
/// Looks up debug source information about a `BytePos`.
pub fn lookup_debug_loc(&self, pos: BytePos) -> DebugLoc {

This comment has been minimized.

Copy link
@bjorn3

bjorn3 Mar 13, 2020

Contributor

Can you please move this and DebugLoc to rustc_codegen_ssa?

bors added a commit that referenced this pull request Mar 15, 2020
Rollup of 7 pull requests

Successful merges:

 - #69357 (Emit 1-based column numbers in debuginfo)
 - #69471 (Remove `sip::Hasher::short_write`.)
 - #69498 (Change "method" to "associated function")
 - #69967 (Remove a few `Rc`s from RegionInferenceCtxt)
 - #69987 (Add self to .mailmap)
 - #69991 (fix E0117 message out of sync)
 - #69993 (Add long error explanation for E0693)

Failed merges:

r? @ghost
bors added a commit that referenced this pull request Mar 15, 2020
Rollup of 7 pull requests

Successful merges:

 - #69357 (Emit 1-based column numbers in debuginfo)
 - #69471 (Remove `sip::Hasher::short_write`.)
 - #69498 (Change "method" to "associated function")
 - #69967 (Remove a few `Rc`s from RegionInferenceCtxt)
 - #69987 (Add self to .mailmap)
 - #69991 (fix E0117 message out of sync)
 - #69993 (Add long error explanation for E0693)

Failed merges:

r? @ghost
@bors bors merged commit 62c0579 into rust-lang:master Mar 15, 2020
4 checks passed
4 checks passed
pr Build #20200226.44 succeeded
Details
pr (Linux mingw-check) Linux mingw-check succeeded
Details
pr (Linux x86_64-gnu-llvm-7) Linux x86_64-gnu-llvm-7 succeeded
Details
pr (Linux x86_64-gnu-tools) Linux x86_64-gnu-tools succeeded
Details
@tmiasko tmiasko deleted the tmiasko:debuginfo-column branch Mar 15, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

8 participants
You can’t perform that action at this time.