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

Alexey Samsonov vonosmas at gmail.com
Tue Jun 2 20:57:58 PDT 2015


Alright, I agree with rationale for this change, and VS integration is nice to have...


================
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));
----------------
Just curious, can you print `file(line)` or `file(line,0)` in VS?

================
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:
> 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?

================
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,
----------------
I find it confusing, that you should pass strip_path_prefix and vs_style in different order, than in the function above.

http://reviews.llvm.org/D10113

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






More information about the llvm-commits mailing list