[PATCH] D37529: [codeview] omit debug locations for nested exprs unless column info enabled

[Reid Kleckner via Phabricator via cfe-commits]

rnk added a subscriber: echristo.
rnk added inline comments.


================
Comment at: clang/lib/CodeGen/CGStmt.cpp:38
 
+InhibitDebugLocation CodeGenFunction::EmitStmtStopPoint(const Stmt *S) {
+  InhibitDebugLocation IDL;
----------------
"Stop point" is a hold-over from the llvm.dbg.stoppoint, which doesn't exist anymore:
http://releases.llvm.org/2.6/docs/SourceLevelDebugging.html#format_common_stoppoint

@echristo renamed CGDebugInfo::EmitStopPoint to EmitLocation long ago, and we should do the same for CodeGenFunction::EmitStopPoint. I'd like to have something like `EmitStmtLocation` and `EmitExprLocation`.


================
Comment at: clang/lib/CodeGen/CGStmt.cpp:45
+  }
+  return IDL;
+}
----------------
Does MSVC accept this? I think it will emit the copy ctor call in an -O0 build.


================
Comment at: clang/lib/CodeGen/CGStmt.cpp:145
 
   case Stmt::IfStmtClass:       EmitIfStmt(cast<IfStmt>(*S));             break;
   case Stmt::WhileStmtClass:    EmitWhileStmt(cast<WhileStmt>(*S));       break;
----------------
Doesn't this end up recursing? Won't InhibitDebugLocation prevent us from applying the inner statement locations?


================
Comment at: clang/test/CodeGenCXX/debug-info-nested-exprs.cpp:12
+
+// NONEST: call i32 @{{.*}}bar{{.*}}, !dbg ![[LOC:[0-9]+]]
+// NONEST: call i32 @{{.*}}baz{{.*}}, !dbg ![[LOC]]
----------------
This is pretty painful to test. :(

If we let ourselves do an asm test, .cv_loc is printed with some nice assembly comments that make this testing easy.


================
Comment at: clang/test/CodeGenCXX/debug-info-nested-exprs.cpp:48
+         (y * z);
+}
----------------
I'd like to see some compound statement tests: for, while, if, regular braces, etc.


https://reviews.llvm.org/D37529