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

[Bob Haarman via Phabricator via cfe-commits]

inglorion added a comment.

@zturner: I am still thinking about your comment about other cases to test. My concern is that there are very many possible combinations.

I'm actually not too concerned about not exactly matching cl's behavior in every single case. The difference in behavior here is us emitting a debug location for an expression that doesn't get its own debug location from cl. In general, I think having more fine-grained information is good, so I don't think differing in this way is a problem. That is, unless we end up breaking functionality in the debugger (Visual Studio). The behavior I know of we can end up breaking this way is step into specific, which appears to require multiple calls that are associated with a single debug location.

I think, at a minimum, the test case should cover a scenario where we would normally like to emit some debug locations, but need to elide them if we want to be compatible with Visual Studio. I also think it makes sense to include some cases where we want the behavior to be the same whether or not we're targeting MS compatibility. That way, we can verify that we aren't throwing away too much information. Beyond that, I feel there are diminishing returns. To avoid going too far down that path, I would like to start with a relatively small test case (as I've done), fix the bug that prompted me to write this code, and then add additional tests if we find out there are other cases where people care strongly about the granularity of the debug locations we emit. Does that sound reasonable?


https://reviews.llvm.org/D37529