[PATCH] IR: Add specialized debug info metadata nodes
Adrian Prantl
aprantl at apple.com
Wed Feb 4 16:17:18 PST 2015
> On Feb 4, 2015, at 3:53 PM, Frédéric Riss <friss at apple.com> wrote:
>
>
>> On Feb 4, 2015, at 3:46 PM, Duncan P. N. Exon Smith <dexonsmith at apple.com> wrote:
>>
>>
>>> On 2015-Feb-04, at 15:37, Frédéric Riss <friss at apple.com> wrote:
>>>
>>>>>
>>>>> - The word 'context' is overloaded: `MDNode::getContext()` already
>>>>> exists, and returns an `LLVMContext&`; `DIDescriptor` uses 'context'
>>>>> to mean "the node that this one is defined inside". I chose the
>>>>> word 'parent' instead of 'context' here. Is this word okay? If
>>>>> not, what about 'scope'? This will be reflected in the assembly
>>>>> changes to come (I'd like the C++ names to match the assembly names,
>>>>> although technically it's not necessary).
>>>>>
>>>>> I'd /probably/ go with scope (we already have scope in the MDLocations, so that seems consistent), but fairly on-the-fence.
>>>>>
>>>>
>>>> Weirdly, I didn't even notice that :). In that case I like 'scope'
>>>> better too. I'll update to that before commit.
>>>
>>> Seems most natural. Can the futur getScope() return something that doesn’t derive from MDScope?
>>
>> I don't think so.
>
> That was my impression also, and it makes it even more appropriate IMHO.
>
Just a few general remarks to throw into the discussion:
- Would it make sense to use something like tablegen to generate the repetitive parts? I’m slightly worried about copy&paste bugs, moderately worried about refactoring it in the future.
As for scopes, there are several things that bug me about the current class hierarchy that we could fix now:
- DIFile should not be a scope (the concept of files is IMO orthogonal to scoping and there is always something more appropriate to put a node into: compile unit, module, namespace)
- DIBasicType should not be scope
- It’s questionable whether a DICompositetype should be a DIDerivedType
- A DISubroutineType should be neither a DIDerivedType nor DIScope
- Using DIDerivedTypes for CV qualifiers is a bit wasteful but it does map nicely to DWARF
otherwise, thanks for doing this!
-- adrian
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150204/126cb4c4/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: classllvm_1_1DIScope__inherit__graph.png
Type: image/png
Size: 28374 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150204/126cb4c4/attachment.png>
More information about the llvm-commits
mailing list