[Lldb-commits] [PATCH] D51661: Print column info in backtraces et al. if available

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

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



================
Comment at: source/Core/Debugger.cpp:123
+#define FILE_AND_LINE                                                          \
+  "{ at ${line.file.basename}:${line.number}${line.column}}"
 #define IS_OPTIMIZED "{${function.is-optimized} [opt]}"
----------------
aprantl wrote:
> clayborg wrote:
> > 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}}"
> > ```
> 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
> ...
> ```
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}}}"
```




================
Comment at: source/Core/FormatEntity.cpp:1824
+        format = entry.printf_format.c_str();
+      s.Printf(format, sc->line_entry.column);
+    }
----------------
return true here


================
Comment at: source/Core/FormatEntity.cpp:1827
+    // Column info is optional, so this always succeeds.
+    return true;
+
----------------
return false here


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D51661