Fix macro call site spans by jseyfried · Pull Request #33749 · rust-lang/rust

jseyfried · 2016-05-20T00:55:44Z

Fix macro call site spans.
r? @nrc

nrc · 2016-05-23T03:13:55Z

LGTM

@DanielJCampbell does this break any of your macro tracing stuff?

DanielJCampbell · 2016-05-23T04:52:21Z

Not that I can see. No failed tests means it passed the tests I had for my stuff, which is a good sign, and I didn't use the original_span method for anything.
Overall better spans is always good, even if I need to go back and tidy my stuff up if I find any issues later.

nrc · 2016-05-24T04:40:34Z

@bors: r+

bors · 2016-05-24T04:40:35Z

📌 Commit 32b9494 has been approved by nrc

bors · 2016-05-24T05:44:23Z

⌛ Testing commit 32b9494 with merge f3449b6...

bors · 2016-05-24T06:53:07Z

💔 Test failed - auto-win-gnu-32-opt-rustbuild

alexcrichton · 2016-05-24T16:05:05Z

@bors: retry

Spurious "filename too long" failure on bots #33844

bors · 2016-05-26T07:53:00Z

⌛ Testing commit 32b9494 with merge c5bbba8...

bors · 2016-05-26T09:52:47Z

💔 Test failed - auto-linux-64-opt-mir

bors · 2016-05-26T18:36:00Z

☔ The latest upstream changes (presumably #33766) made this pull request unmergeable. Please resolve the merge conflicts.

jseyfried · 2016-05-27T00:28:47Z

Rebased. @bors r=nrc

bors · 2016-05-27T00:28:49Z

📌 Commit e77a219 has been approved by nrc

bors · 2016-05-27T00:29:10Z

💡 This pull request was already approved, no need to approve it again.

There's another pull request that is currently being tested, blocking this pull request: Add AST validation pass and move some checks to it #33794

bors · 2016-05-27T00:29:10Z

📌 Commit e77a219 has been approved by nrc

bors · 2016-05-27T00:55:41Z

⌛ Testing commit e77a219 with merge 5b1bd32...

@nrc

Fix macro call site spans Fix macro call site spans. r? @nrc

bors · 2016-05-27T03:03:58Z

💔 Test failed - auto-linux-64-opt-mir

jseyfried · 2016-05-27T11:23:18Z

@bors retry

bors · 2016-05-27T16:21:04Z

⌛ Testing commit e77a219 with merge e8ab466...

bors · 2016-05-27T18:21:09Z

💔 Test failed - auto-linux-64-opt-mir

alexcrichton · 2016-05-27T18:22:51Z

(that error looks like it may be legit)

jseyfried · 2016-05-28T10:22:33Z

I'm having trouble reproducing the error, but I think the above commit will fix it -- r? @nrc

nrc · 2016-05-30T14:10:35Z

r? @michaelwoerister for the last commit (r=me for the rest). If the #break comments are where the debugger should break, that looks wrong, but I have no idea what is going on here.

jseyfried · 2016-06-13T04:35:19Z

@michaelwoerister
The incorrect call site spans that this PR fixes cause very confusing diagnostics.

Since debuginfo for macros is already known to be buggy, perhaps we could just land this with the workaround in the last commit?

Another option would be to always use call site spans for debuginfo as described in this comment. While I believe this would fix the bugs I linked and allow this PR to land without the workaround, it would make it impossible to break on a line in a macro definition (in particular, we would have to remove this test).

michaelwoerister · 2016-06-13T04:53:55Z

Oh, I forgot about this PR. I'll take a look at it tomorrow.

michaelwoerister · 2016-06-13T14:25:31Z

OK, so I won't get into debuginfo vs macros here -- I'd need to put a lot more thought into that and assess the current situation. From what I can see here, things don't get considerably worse, right?

As far as the test case goes: There shouldn't even be a macro invocation here -- in fact, it had been a regular function call before someone changed it to account for the called function being moved, as far as I can tell. We just need something that allows us to reliably set a breakpoint at. I'd suggest just following the pattern of many other debuginfo test cases: Add an empty function fn zzz() { () } and call it wherever a breakpoint is needed. That should allow the #break directive to work reliably.

… debuginfo test

jseyfried · 2016-06-13T23:06:27Z

From what I can see here, things don't get considerably worse, right?

Right. I believe things can only get worse when there is a macro-expanded procedural macro invocation (in this case, format!("")). If format!("") were invoked directly or if format were somehow defined with macro_rules!, the test would pass after this PR or would already be broken (respectively).

I'd suggest just following the pattern of many other debuginfo test cases: Add an empty function fn zzz() { () } and call it wherever a breakpoint is needed.

Makes sense, done.

michaelwoerister · 2016-06-14T02:58:34Z

@bors r=nrc

Excellent :)

bors · 2016-06-14T02:58:36Z

📌 Commit 2477341 has been approved by nrc

bors · 2016-06-14T04:07:30Z

⌛ Testing commit 2477341 with merge bf84f4e...

@nrc

Fix macro call site spans Fix macro call site spans. r? @nrc

bors · 2016-06-14T07:24:31Z

☀️ Test successful - auto-linux-32-nopt-t, auto-linux-32-opt, auto-linux-32cross-opt, auto-linux-64-cargotest, auto-linux-64-cross-armhf, auto-linux-64-cross-armsf, auto-linux-64-cross-freebsd, auto-linux-64-cross-netbsd, auto-linux-64-debug-opt, auto-linux-64-nopt-t, auto-linux-64-opt, auto-linux-64-opt-mir, auto-linux-64-opt-rustbuild, auto-linux-64-x-android-t, auto-linux-cross-opt, auto-linux-musl-64-opt, auto-mac-32-opt, auto-mac-64-nopt-t, auto-mac-64-opt, auto-mac-64-opt-rustbuild, auto-mac-cross-ios-opt, auto-win-gnu-32-nopt-t, auto-win-gnu-32-opt, auto-win-gnu-32-opt-rustbuild, auto-win-gnu-64-nopt-t, auto-win-gnu-64-opt, auto-win-msvc-32-cross-opt, auto-win-msvc-32-opt, auto-win-msvc-64-cargotest, auto-win-msvc-64-opt, auto-win-msvc-64-opt-mir, auto-win-msvc-64-opt-rustbuild

@nrc

Fix `include!()`s inside `asm!()` invocations Fixes#34812, a regression caused by #33749 that was not fixed in #34450. r? @nrc

rust-highfive assigned nrc May 20, 2016

durka mentioned this pull request May 21, 2016
missing note in compiler error when expanding macro #33774
Closed

jseyfried force-pushed the fix_call_site_span branch from 32b9494 to e77a219Compare May 27, 2016 00:21

bors added a commit that referenced this pull request May 27, 2016
Auto merge of#33749- jseyfried:fix_call_site_span, r=nrc
5b1bd32
Fix macro call site spans Fix macro call site spans. r? @nrc

jseyfried added 2 commits May 28, 2016 05:22

Fix macro call site spans
4259fba

Fix fallout in tests
cc36e38

jseyfried force-pushed the fix_call_site_span branch from e77a219 to cc36e38Compare May 28, 2016 05:24

rust-highfive assigned michaelwoerister and unassigned nrc May 30, 2016

jseyfried mentioned this pull request Jun 10, 2016
wrong line number for function in DWARF #33146
Closed

Replace println!(""); invocations with calls to a dummy function in…
2477341
… debuginfo test

jseyfried force-pushed the fix_call_site_span branch from 1458996 to 2477341Compare June 13, 2016 22:58

bors added a commit that referenced this pull request Jun 14, 2016
Auto merge of#33749- jseyfried:fix_call_site_span, r=nrc
bf84f4e
Fix macro call site spans Fix macro call site spans. r? @nrc

bors merged commit 2477341 into rust-lang:masterJun 14, 2016

bors mentioned this pull request Jun 14, 2016
libsyntax: miscellaneous cleanup #33943
Merged

jseyfried mentioned this pull request Jun 24, 2016
Possible regression at nightly-2016-06-15 for include_str! macro #34431
Closed

This was referenced Jul 14, 2016
Fix include!()s inside asm!() invocations #34816
Merged
trace_macros-gate.rs fails distcheck #34822
Closed

bors added a commit that referenced this pull request Jul 16, 2016
Auto merge of#34816- jseyfried:fix_include_path, r=nrc
eb196dc
Fix `include!()`s inside `asm!()` invocations Fixes#34812, a regression caused by #33749 that was not fixed in #34450. r? @nrc

Uh oh!

Uh oh!

Conversation

jseyfried commented May 20, 2016• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nrc commented May 23, 2016

Uh oh!

DanielJCampbell commented May 23, 2016• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nrc commented May 24, 2016

Uh oh!

bors commented May 24, 2016

Uh oh!

bors commented May 24, 2016

Uh oh!

bors commented May 24, 2016

Uh oh!

alexcrichton commented May 24, 2016

Uh oh!

bors commented May 26, 2016

Uh oh!

bors commented May 26, 2016

Uh oh!

bors commented May 26, 2016

Uh oh!

jseyfried commented May 27, 2016• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bors commented May 27, 2016

Uh oh!

bors commented May 27, 2016

Uh oh!

bors commented May 27, 2016

Uh oh!

bors commented May 27, 2016

Uh oh!

bors commented May 27, 2016

Uh oh!

jseyfried commented May 27, 2016

Uh oh!

bors commented May 27, 2016

Uh oh!

bors commented May 27, 2016

Uh oh!

alexcrichton commented May 27, 2016

Uh oh!

jseyfried commented May 28, 2016• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nrc commented May 30, 2016

Uh oh!

jseyfried commented Jun 13, 2016

Uh oh!

michaelwoerister commented Jun 13, 2016

Uh oh!

michaelwoerister commented Jun 13, 2016

Uh oh!

jseyfried commented Jun 13, 2016

Uh oh!

michaelwoerister commented Jun 14, 2016

Uh oh!

bors commented Jun 14, 2016

Uh oh!

bors commented Jun 14, 2016

Uh oh!

bors commented Jun 14, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

jseyfried commented May 20, 2016•
edited
Loading

DanielJCampbell commented May 23, 2016•
edited
Loading

jseyfried commented May 27, 2016•
edited
Loading

jseyfried commented May 28, 2016•
edited
Loading