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

[David Blaikie]

On Tue, Feb 3, 2015 at 4:19 PM, Zachary Turner <zturner at google.com> wrote:

> Yes.  Although it seems there's no way to do this without manually
> checking in binary files (since only MSVC can generate PDBs).
>

That's fine - we do the same thing for testing dwarfdump, objdump, etc.
Take a look at the existing tests there for some inspiration - you'll find
the source checked in nearby for reference and to allow the binaries to be
modified (add new features, recompile, checkin the binary and the new parts
of the test to test the new dumping feature you're adding, etc).

- David


>
>
> On Tue Feb 03 2015 at 4:11:49 PM Frédéric Riss <friss at apple.com> wrote:
>
>> On Feb 3, 2015, at 3:52 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.
>>
>>
>> I’m following this from afar, I have just one question: do you plan on
>> submitting tests for the PDB reading? Of course they can be run only on
>> Windows, but it’s certainly worth having at least that automated coverage.
>>
>> Fred
>>
>> 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/
>>>>>>
>>>>>>
>>>>>>  _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>
>>
>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150203/27283be7/attachment.html>