[PATCH] [sanitizer_common] Added VS-style output for source locations

[Filipe Cabecinhas]

================
Comment at: lib/sanitizer_common/sanitizer_flags.inc:149
@@ -148,1 +148,3 @@
             "Print inlined frames in stacktraces. Defaults to true.")
+COMMON_FLAG(bool, symbolize_vs_style, false,
+            "Print file locations in Visual Studio style (e.g: "
----------------
This should probably default to SANITIZER_WINDOWS instead of false.
I think it would be the option that better matches what a user of that platform would expect, no?

================
Comment at: lib/sanitizer_common/sanitizer_stacktrace_printer.cc:119
@@ -118,1 +118,3 @@
+                          bool vs_style) {
+  bool print_vs_style = vs_style && line > 0 && column > 0;
   buffer->append("%s", StripPathPrefix(file, strip_path_prefix));
----------------
samsonov wrote:
> samsonov wrote:
> > Just curious, can you print `file(line)` or `file(line,0)` in VS?
> I'd rather handle VS case separately
>   if (vs_style && line > 0 && column > 0) {
>     buffer->append("%s(%d,%d)", ....);
>     return;
>   }
> 
> BTW, what is the expected behavior if, say, vs_style is true, but column is unknown. Is it really OK to fallback to traditional source location?
Yes.

I tested these variations:
  printf(__FILE__ ": hello world!\n");
  printf(__FILE__ "(0,0): hello world!\n");
  printf(__FILE__ "(%d,0): hello world!\n", __LINE__);
  printf(__FILE__ "(%d): hello world!\n", __LINE__);
  printf(__FILE__ "(%d,17): hello world!\n", __LINE__);

Both UNIX-style ("file:3:42") and the first two in that set will select the first line in the file (absence of line info).
The other three all select their respective lines.

I think the easiest option would be to do a variation of what you mentioned:
  if (vs_style && line > 0) {
    buffer->append("%s(%d,%d)", ....);
    return;
  } // Fallback to UNIX-style if there's no line info
(not caring about column info and printing 0 if absent)

The "prettiest" (read: less confusing output) might be:
  if (vs_style && line > 0) {
    buffer->append("%s(%d", ...);
    if (column > 0)
      buffer->append(",%d)", ....);
    else
      buffer->append(")", ....);
    return;
  } // Fallback to UNIX-style if there's no line info
(Only printing column info if we actually have column info)

I'm ok with either one (or even not having the else clause and splitting "%d" from ")"). Let me know which you would prefer.

================
Comment at: lib/sanitizer_common/sanitizer_stacktrace_printer.h:55
@@ -53,2 +54,3 @@
 
 void RenderSourceLocation(InternalScopedString *buffer, const char *file,
+                          int line, int column, const char *strip_path_prefix,
----------------
samsonov wrote:
> I find it confusing, that you should pass strip_path_prefix and vs_style in different order, than in the function above.
Will change.

http://reviews.llvm.org/D10113

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/