[llvm] r346429 - [NativePDB] Higher fidelity reconstruction of AST from Debug Info.

[David Blaikie via llvm-commits]

Maybe we do have coverage but not through this member function - if the
code for nested classes that is exercised by LLVM lit or unit tests
(assuming it's tested somewhere) could be changed to use this member
function, then it'd be tested & I'd be good with it.

Yeah, I don't mind what form the tests take - but just asking the general
"this is a change to LLVM, does it/can it/should it have a test in LLVM?"

On Mon, Nov 12, 2018 at 11:10 AM Zachary Turner <zturner at google.com> wrote:

> I believe we do already have test coverage for nested classes, i was just
> adding this helper method so I didn’t have to write the long if statement.
> But if we don’t have coverage for nested types, then maybe a lit test would
> be better?
> On Mon, Nov 12, 2018 at 11:04 AM David Blaikie <dblaikie at gmail.com> wrote:
>
>> This is, 'technically', a separate patch in some sense from the LLD part
>> - changes to LLVM usually merit tests in LLVM to ensure they work/don't
>> regress/etc without needing to checkout & test other subprojects (like lld).
>>
>> On Mon, Nov 12, 2018 at 11:02 AM Zachary Turner <zturner at google.com>
>> wrote:
>>
>>> This one specific function, or the patch as a whole? I thought the patch
>>> had pretty good non-unit-test coverage
>>> On Mon, Nov 12, 2018 at 10:36 AM David Blaikie <dblaikie at gmail.com>
>>> wrote:
>>>
>>>> Could this use/be tested with a unit test, perhaps?
>>>>
>>>> On Thu, Nov 8, 2018 at 10:52 AM Zachary Turner via llvm-commits <
>>>> llvm-commits at lists.llvm.org> wrote:
>>>>
>>>>> Author: zturner
>>>>> Date: Thu Nov  8 10:50:11 2018
>>>>> New Revision: 346429
>>>>>
>>>>> URL: http://llvm.org/viewvc/llvm-project?rev=346429&view=rev
>>>>> Log:
>>>>> [NativePDB] Higher fidelity reconstruction of AST from Debug Info.
>>>>>
>>>>> In order to accurately put a type into the correct location in the AST
>>>>> we construct from debug info, we need to be able to determine what
>>>>> DeclContext (namespace, global, nested class, etc) that it goes into.
>>>>> PDB doesn't contain this mapping.  It does, however, contain the
>>>>> reverse
>>>>> mapping.  That is, for a given class type T, you can determine all
>>>>> classes Q1, Q2, ..., Qn that are nested inside of T.  We need to know,
>>>>> for a given class type Q, what type T is it nested inside of.
>>>>>
>>>>> This patch builds this map as a pre-processing step when we first
>>>>> load the PDB by scanning every type.  Initial tests show that while
>>>>> this can be slow in debug builds of LLDB, it is quite fast in release
>>>>> builds (less than 2 seconds for a ~1GB PDB, and it only needs to happen
>>>>> once).
>>>>>
>>>>> Furthermore, having this pre-processing step in place allows us to
>>>>> repurpose it for building up other kinds of indexing to it down the
>>>>> line.  For the time being, this gives us very accurate reconstruction
>>>>> of the DeclContext hierarchy.
>>>>>
>>>>> Differential Revision: https://reviews.llvm.org/D54216
>>>>>
>>>>> Modified:
>>>>>     llvm/trunk/include/llvm/DebugInfo/CodeView/TypeRecord.h
>>>>>
>>>>> Modified: llvm/trunk/include/llvm/DebugInfo/CodeView/TypeRecord.h
>>>>> URL:
>>>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/DebugInfo/CodeView/TypeRecord.h?rev=346429&r1=346428&r2=346429&view=diff
>>>>>
>>>>> ==============================================================================
>>>>> --- llvm/trunk/include/llvm/DebugInfo/CodeView/TypeRecord.h (original)
>>>>> +++ llvm/trunk/include/llvm/DebugInfo/CodeView/TypeRecord.h Thu Nov  8
>>>>> 10:50:11 2018
>>>>> @@ -429,6 +429,10 @@ public:
>>>>>      return (Options & ClassOptions::ForwardReference) !=
>>>>> ClassOptions::None;
>>>>>    }
>>>>>
>>>>> +  bool containsNestedClass() const {
>>>>> +    return (Options & ClassOptions::ContainsNestedClass) !=
>>>>> ClassOptions::None;
>>>>> +  }
>>>>> +
>>>>>    bool isScoped() const {
>>>>>      return (Options & ClassOptions::Scoped) != ClassOptions::None;
>>>>>    }
>>>>>
>>>>>
>>>>> _______________________________________________
>>>>> llvm-commits mailing list
>>>>> llvm-commits at lists.llvm.org
>>>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>>>>
>>>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20181112/f94d721d/attachment.html>