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

[Zachary Turner via llvm-commits]

The tests in llvm just get the whole flags enum and then format it. So if
this flag is set in the output, you’ll see it. But it doesn’t seem easy to
make that formatting function go through this method. That said, I think
the important thing to test is that this flag properly gets set in the enum
in the correct condition. So the current test tests that I think. But this
helper method seems obviously correct (and pretty trivial) and not worth
adding a special test for
On Mon, Nov 12, 2018 at 11:36 AM David Blaikie <dblaikie at gmail.com> wrote:

> 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/b3b31ca4/attachment.html>