This is an archive of the discontinued LLVM Phabricator instance.

Print column info in backtraces et al. if available
ClosedPublic

Authored by aprantl on Sep 4 2018, 3:49 PM.

Details

Summary

This patch allows LLDB to print column info in backtraces et al. if available, which is useful when the backtrace contains a frame like the following:

f(can_crash(0), can_crash(1));

Diff Detail

Repository
rL LLVM

Event Timeline

aprantl created this revision.Sep 4 2018, 3:49 PM

We shouldn't have the colon character be part of the ${line.column} formatting itself. See inlined comments.

source/Core/Debugger.cpp
123 ↗(On Diff #163937)

Add the colon between the ${line.number} and ${line.column} here, not as part of the "${line.column}" formatting itself.

"{ at ${line.file.basename}:${line.number}:${line.column}}"
source/Core/FormatEntity.cpp
1821 ↗(On Diff #163937)

Remove the colon character from format.

clayborg requested changes to this revision.Sep 5 2018, 9:42 AM
This revision now requires changes to proceed.Sep 5 2018, 9:42 AM
aprantl added inline comments.Sep 5 2018, 10:11 AM
source/Core/Debugger.cpp
123 ↗(On Diff #163937)

But then the colon would be printed even if there is no column. Some frames may not have column info. I'd like to have it look like this:

frame #0 at hello.c:100:42
frame #1 at nocolumns.cpp:50
...

See my inlined comments about returning true and false correctly for the column and the correction to the format string

source/Core/Debugger.cpp
123 ↗(On Diff #163937)

If we return true/false correctly as detailed in inline comments, you just add an extra scope {} around the ":${line.column}" and it will not be printed if anything inside the extra scope returns false:

"{ at ${line.file.basename}:${line.number}{:${line.column}}}"
source/Core/FormatEntity.cpp
1824 ↗(On Diff #163937)

return true here

1826 ↗(On Diff #163937)

Remove this comment

1827 ↗(On Diff #163937)

return false here

aprantl updated this revision to Diff 164104.Sep 5 2018, 1:09 PM

Address review feedback.

Thanks you, I was wondering how to properly implement an optional element!

clayborg accepted this revision.Sep 5 2018, 4:46 PM
This revision is now accepted and ready to land.Sep 5 2018, 4:46 PM
This revision was automatically updated to reflect the committed changes.