<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Feb 6, 2015 at 1:20 PM, Duncan P. N. Exon Smith <span dir="ltr"><<a href="mailto:dexonsmith@apple.com" target="_blank">dexonsmith@apple.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><span class=""><br>
> On 2015-Feb-06, at 10:36, David Blaikie <<a href="mailto:dblaikie@gmail.com">dblaikie@gmail.com</a>> wrote:<br>
><br>
><br>
><br>
> On Wed, Feb 4, 2015 at 4:17 PM, Adrian Prantl <<a href="mailto:aprantl@apple.com">aprantl@apple.com</a>> wrote:<br>
><br>
>> On Feb 4, 2015, at 3:53 PM, Frédéric Riss <<a href="mailto:friss@apple.com">friss@apple.com</a>> wrote:<br>
>><br>
>><br>
>>> On Feb 4, 2015, at 3:46 PM, Duncan P. N. Exon Smith <<a href="mailto:dexonsmith@apple.com">dexonsmith@apple.com</a>> wrote:<br>
>>><br>
>>><br>
>>>> On 2015-Feb-04, at 15:37, Frédéric Riss <<a href="mailto:friss@apple.com">friss@apple.com</a>> wrote:<br>
>>>><br>
>>>>>><br>
>>>>>> - The word 'context' is overloaded: `MDNode::getContext()` already<br>
>>>>>> exists, and returns an `LLVMContext&`; `DIDescriptor` uses 'context'<br>
>>>>>> to mean "the node that this one is defined inside". I chose the<br>
>>>>>> word 'parent' instead of 'context' here. Is this word okay? If<br>
>>>>>> not, what about 'scope'? This will be reflected in the assembly<br>
>>>>>> changes to come (I'd like the C++ names to match the assembly names,<br>
>>>>>> although technically it's not necessary).<br>
>>>>>><br>
>>>>>> I'd /probably/ go with scope (we already have scope in the MDLocations, so that seems consistent), but fairly on-the-fence.<br>
>>>>>><br>
>>>>><br>
>>>>> Weirdly, I didn't even notice that :). In that case I like 'scope'<br>
>>>>> better too. I'll update to that before commit.<br>
>>>><br>
>>>> Seems most natural. Can the futur getScope() return something that doesn’t derive from MDScope?<br>
>>><br>
>>> I don't think so.<br>
>><br>
>> That was my impression also, and it makes it even more appropriate IMHO.<br>
>><br>
><br>
> Just a few general remarks to throw into the discussion:<br>
><br>
> - 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.<br>
><br>
</span>> <classllvm_1_1DIScope__inherit__graph.png><br>
<span class="">><br>
> As for scopes, there are several things that bug me about the current class hierarchy that we could fix now:<br>
> - 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)<br>
><br>
> IIRC we do this when # line directives change the file-name part-way through. But I could be wrong... only vague idea.<br>
><br>
> - DIBasicType should not be scope<br>
><br>
> Part of this is the ability to treat all types the same - multiple inheritance might be an alternative, but without that we want to treat all types equally in some codepaths, but then only some types are valid scopes, etc. - so there's a few different axes on which we want to refer to these things. It's tricky. But certainly worth thinking about.<br>
<br>
</span>LLVM-style RTTI doesn't handle multiple inheritance, which restricts<br>
us somewhat here.<br></blockquote><div><br></div><div>It definitely does. Check out clang's DeclContext vs. Decl multiple inheritance.</div><div><br></div><div>I'll update <a href="http://llvm.org/docs/HowToSetUpLLVMStyleRTTI.html#the-contract-of-classof">http://llvm.org/docs/HowToSetUpLLVMStyleRTTI.html#the-contract-of-classof</a> to mention this (that is actually the original reason I added that section, but it appears I forgot to add a concrete reference to MI and in particular DeclContext vs. Decl).</div><div><br></div><div>(I haven't really been following the discussion as to what MI was being proposed to be used for, but don't let the LLVM-style RTTI block you from considering it as a possibility)</div><div><br></div><div>-- Sean Silva</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<br>
I imagine having a hierarchy like:<br>
<br>
DIScopeBase<br>
-> DIScope<br>
-> DINamespace<br>
-> DISubprogram<br>
-> DIType<br>
-> DIBasicType<br>
-> DIDerivedType<br>
-> DITypeScope<br>
-> DICompositeType<br>
<br>
would get us most of the way there.<br>
<br>
Pointers can be `DIScopeBase`. Call-sites that care about scopes<br>
can check:<br>
<br>
assert(isa<DIScope>(N) || isa<DITypeScope>(N));<br>
<br>
(We can add `DIScopeBase::isValidScope()` or something to do this.)<br>
<br>
Or maybe we don't even need a `DIScopeBase`.<br>
<span class="im"><br>
><br>
> - It’s questionable whether a DICompositetype should be a DIDerivedType<br>
> - A DISubroutineType should be neither a DIDerivedType nor DIScope<br>
> - Using DIDerivedTypes for CV qualifiers is a bit wasteful but it does map nicely to DWARF<br>
><br>
> otherwise, thanks for doing this!<br>
> -- adrian<br>
<br>
<br>
</span><div class=""><div class="h5">_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
</div></div></blockquote></div><br></div></div>