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

[Adrian McCarthy via llvm-commits]

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?


https://reviews.llvm.org/D24014