[PATCH] D29833: Improve the API of DILocation::getMergedLocation()

[Adrian Prantl via llvm-commits]

> On Feb 23, 2017, at 9:13 AM, David Blaikie <dblaikie at gmail.com> wrote:
> 
> You were suggesting that the nice thing about the former was that the user couldn't fail to have a debug location:
> 
> "The primary benefit of having the API on Instruction is that we can make it impossible to accidentally create a locationless call instruction in a function with debuginfo (which would crash/assert the backend when creating inline scopes)."
> 
> Which I'm a bit fuzzy on - if a new call instruction is created it'd still have no location so I'm not sure this choice of API provides extra defense against that mistake.
> 
> If, today, an existing call instruction is moved then the location zero'd out - it's going to need work to replace the zeroing with merging no matter whether the API for that merging is on DebugLoc or Instruction.

Agreed.

> 
> So I guess coming back to your point: could you explain in more detail how having the API on Instruction provides a stronger guarantee/safety than having it elsewhere? I'm not clear on what that's defending against.

I want to make it harder to accidentally assign a null DebugLoc to a call. Currently getMergedDebugLoc() returns a null location (to save space in the line table) and at each call site we have to write extra code to create a line:0 location if the instruction we are creating the merged location for is a call. See https://reviews.llvm.org/D29833 <https://reviews.llvm.org/D29833>, lib/Transforms/Utils/SimplifyCFG.cpp:1280 for an example — though upon closer inspection the original code was just keeping the original location of the call instead of assigning a line:0 location.
I though it would be nice to hide this detail in the API, but this really makes it look like we want to decide how to handle call instructions (assign line:0, keep original loc) on a case-by-case basis.

thanks,
adrian
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170223/877063b0/attachment.html>