r177164 - Force column info only for direct inlined functions. This should strike

[David Blaikie]

On Fri, Mar 15, 2013 at 11:31 AM, Adrian Prantl <aprantl at apple.com> wrote:
>
> On Mar 15, 2013, at 11:26 AM, David Blaikie <dblaikie at gmail.com> wrote:
>
>> On Fri, Mar 15, 2013 at 10:46 AM, Adrian Prantl <aprantl at apple.com> wrote:
>>>
>>> On Mar 15, 2013, at 10:25 AM, David Blaikie <dblaikie at gmail.com> wrote:
>>>
>>>> On Fri, Mar 15, 2013 at 10:09 AM, Adrian Prantl <aprantl at apple.com> wrote:
>>>>> Author: adrian
>>>>> Date: Fri Mar 15 12:09:05 2013
>>>>> New Revision: 177164
>>>>>
>>>>> URL: http://llvm.org/viewvc/llvm-project?rev=177164&view=rev
>>>>> Log:
>>>>> Force column info only for direct inlined functions. This should strike
>>>>> the balance between expected behavior and compatibility with the gdb
>>>>> testsuite.
>>>>>
>>>>> (GDB gets confused if we break an expression into multiple debug
>>>>> stmts so we enable this behavior only for inlined functions. For the
>>>>> full experience people can still use -gcolumn-info.)
>>>>
>>>> I'm not sure I understand how this will address the issue... Perhaps
>>>> I'm misunderstanding something about this change. I have a few
>>>> questions/uncertainties:
>>>
>>>> 1) You can't really determine in the frontend if a function will
>>>> actually be inlined - trying to predicate debug info on that seems
>>>> like we'll get surprising/varying debug behavior based on whether the
>>>> backend chooses to inline the function
>>>
>>> That is correct. My thinking was that the backend would only inline at >O0 and if you need more precise debug info you could always use -gcolumn-info. But for those cases where function is inlined at O0 (which happens for inline-attributed functions and is done by the frontend) users will get the expected behavior.
>>> This is not very elegant, but I’m trying to find a middle ground between not breaking gdb’s testsuite and fixing the debug experience for inlined functions.
>>
>> Sure - I'm not suggesting that we should not fix the original bug you
>> set out to fix, just that the solution that Eric & I were
>> discussing/suggesting wasn't what you've implemented here. I thought
>> what we were discussing was changing the backend to not emit separate
>> line table entries when the line is the same but the column is
>> different. This would be consistent with the debug experience users
>> expect (given GDB's lack of column information) in all cases, no
>> matter what things were inlined or not inlined, if I'm not mistaken.
>
> As I mentioned earlier, the problem is that for inlined functions we need to do exactly that otherwise a breakpoint at the inlined function (appearing twice on one line) would be hit only once.

Still feels  wrong in some way - mostly because of the inlining
possibilities (both not inlining when your frontend heuristic would
indicate that it does, and inlining when it doesn't), even at -O0 (we
still do inlining, even on non-always_inline functions, at -O0 - for
simple functions like get/set, etc)

FWIW, GCC does exhibit the same behavior you described when it inlines
a function (if I use the func(foo(), bar()) example & make foo/bar
always_inline, then stepping through goes foo/func call/bar and
stepping through without the always_inline goes foo/bar)

I would like this to be done right so it behaves appropriately
regardless of whether the backend chooses to inline the foo/bar calls.