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

[Duncan P. N. Exon Smith]

> On 2015-Apr-10, at 10:24, David Blaikie <dblaikie at gmail.com> wrote:
> 
> 
> 
> 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?

Yup, makes sense.  I think where I went wrong was that currently we
*do* get different line table entries (sounds like that's a bug/missed
optimization?).

But using column info makes sense, right?  I'll write a FIXME and file
a PR for that (sounds like maybe you guys need to update tools on your
end?).

>  
> 
> 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?)
> 
>