[PATCH] D24014: [codeview] Add new directives to record inlined call site line info

Adrian McCarthy via llvm-commits llvm-commits at lists.llvm.org
Tue Aug 30 11:55:23 PDT 2016

amccarth added a comment.

I'm curious if you tried the example in your `cv-inline-linetable-unlikely` test to see how the VS debugger does with the improved inline information.

Overall, this looks good.

Comment at: include/llvm/MC/MCCodeView.h:112
@@ +111,3 @@
+  /// function, or this is an inlined call site, and this is the parent function
+  /// id plus one.
+  unsigned ParentFuncIdPlusOne = 0;
This comment is a bit muddled and hard to understand.  How about starting with the typical case:

    /// ParentFuncIdPlusOne is the parent function ID plus one.  If this is a top-level function and
    /// there is no parent, then ParentFuncIdPlusOne is FunctionSentinel.

I'm still not sure what zero means.  What specifically is unallocated?

Comment at: include/llvm/MC/MCCodeView.h:133
@@ +132,3 @@
+  enum : unsigned { FunctionSentinel = ~0U };
It would be nice to put this closer to `ParentFuncIdPlusOne`.

Comment at: include/llvm/MC/MCCodeView.h:162
@@ +161,3 @@
+                               unsigned IAFile, unsigned IALine,
+                               unsigned IACol);
What does the IA- prefix mean in this context?

I'm concerned about a method with five parameters of the same type--seems very easy to call with arguments in the wrong order.

Is the return a simple true-mean-success/false-means-failure?


More information about the llvm-commits mailing list