[PATCH] WIP: AddDiscriminators: Check more carefully for equivalent locations

[David Blaikie]

On Thu, Apr 9, 2015 at 5:26 PM, Duncan P. N. Exon Smith <
dexonsmith at apple.com> wrote:

> Stop using `DILocation::atSameLineAs()` to check whether two locations
> are equivalent.  This check is too weak: it ignores any column info that
> might be present, and ignores the scope/inlined-at chains.
>

I don't think we can use the scope/inlined-at can be used here (but, as
Eric said on IRC, we might not have testing for it) - we use the scope
information to build the scope hierarchy in the debug_info, but it doesn't
affect the line table at all. So we could have two debug locations with the
same line/file/column but different scopes, yet the same basic block
(something as simple as {} can introduce a scope without introducing a new
basic block, of course, or a do-while loop would still have a contiguous
range but be two basic blocks) and thus they'd coalesce to the same line
table entry when they should be separate blocks, potentially.

Does that make sense?


>
> Instead, just compare the pointers for equality.  If the pointers are
> different here, we'll get different entries in the line table already.
>
> I haven't updated testcases yet (there's one each failing in LLVM and
> clang that would need to be rewritten), but basically: is there any
> reason to add a discriminator if we'd *already* be getting different
> entries in the line table?  (Is there a broken consumer we need to
> support that relies on the discriminator field?)
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150410/6c1d5972/attachment.html>