[PATCH] Initial pass at API design for DebugInfo/PDB

Zachary Turner zturner at google.com
Tue Feb 3 15:54:33 PST 2015


I suppose I could also finish off the rest of the concrete symbol types
before going in.

On Tue Feb 03 2015 at 3:52:13 PM Zachary Turner <zturner at google.com> wrote:

> Fair enough, it's not a huge deal either way.  I've made this and some of
> the other suggested changes locally.
>
> I'm a little bit more comfortable with this design I have now, especially
> now that it works directly with llvm::dyn_cast<> and llvm::isa<>.  Anyone
> have any major objections or concerns?  I will probably try to go in with
> this sometime tomorrow if not.
>
> On Tue Feb 03 2015 at 3:29:14 PM David Blaikie <dblaikie at gmail.com> wrote:
>
>> On Tue, Feb 3, 2015 at 3:25 PM, Zachary Turner <zturner at google.com>
>> wrote:
>>
>>> I was thinking it might still be useful for people who knew what they
>>> were doing and who wanted to write some implementation-specific code to be
>>> able to get at the native native interface.  Like in this case a raw COM
>>> pointer.  In theory this shouldn't ever be necessary, but it's hard to say
>>> without knowing anything about implementation strategies other than DIA how
>>> much compatibility someone would be able to achieve, and if there ever
>>> might be things that are only possible to implement in terms of one API but
>>> not the other.  So I didn't want to prematurely remove the possibility to
>>> get at the native interface, for example by writing static_cast<DIASymbol*>(
>>> PDBSymbol::getRawSymbol())->someDIAOnlyMethod().
>>>
>>
>> I tend to err on the other side - we can always add more API if we find a
>> use for it. No need to pay for things we're not using in the interim. But
>> ultimately up to you.
>>
>>
>> - David
>>
>>
>>>
>>> On Tue Feb 03 2015 at 3:19:11 PM David Blaikie <dblaikie at gmail.com>
>>> wrote:
>>>
>>>> On Tue, Feb 3, 2015 at 3:11 PM, Zachary Turner <zturner at google.com>
>>>> wrote:
>>>>
>>>>> ================
>>>>> Comment at: include/llvm/DebugInfo/PDB/IPDBRawSymbol.h:193
>>>>> @@ +192,3 @@
>>>>> +  virtual bool isVirtualInheritance() const = 0;
>>>>> +  virtual bool isVolatileType() const = 0;
>>>>> +};
>>>>> ----------------
>>>>> dblaikie wrote:
>>>>> > How're these functions going to communicate failure?
>>>>> Undefined in theory, probably false in practice.  You shouldn't call
>>>>> methods on the raw interface unless you know it's a valid method.
>>>>>
>>>>> For the purposes of a dumper who wanted to detect unknown / unexpected
>>>>> fields, that knowledge could all be encapsulated in the implementation of
>>>>> the raw interface.  For example, you could have PDBSymbol::dump() which
>>>>> calls RawSymbol->dump(), and that particular implementation can go to the
>>>>> native API instead of calling the friendly accessors.
>>>>>
>>>>
>>>> Ah - that's a bit different from what I was thinking from our previous
>>>> discussion.
>>>>
>>>> If it's undefined behavior, then it might be appropriate to hide
>>>> PDBRawSymbol from users entirely - they might as well be casting down to
>>>> the specific type and using those functions instead, perhaps? (I was
>>>> thinking clients would be able to use PDBRawSymbol and just call all the
>>>> functions and swallow the failures when they would call the wrong functions)
>>>>
>>>>
>>>> - David
>>>>
>>>>
>>>>>
>>>>> http://reviews.llvm.org/D7356
>>>>>
>>>>> EMAIL PREFERENCES
>>>>>   http://reviews.llvm.org/settings/panel/emailpreferences/
>>>>>
>>>>>
>>>>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150203/ec058798/attachment.html>


More information about the llvm-commits mailing list