[llvm] r189495 - PR16995: DebugInfo: Don't overwrite existing member lists when adding template arguments

[Eric Christopher]

> Yes, the first part provides the "don't overwrite existing member list
> when adding template arguments", see the call site in Clang,
> CGDebugInfo.cpp:2270:
>
> RealDecl.setTypeArray(llvm::DIArray(),
> CollectCXXTemplateParams(TSpecial, DefUnit));
>
> When we CollectCXXTemplateParams we might actually end up discovering
> some of the members of 'RealDecl' (such as static members, see the
> test case in the referenced PR) that we then overwrite to empty when
> setTypeArray is called.

Fun. Seems error prone in some way, but...

>
>> I do wish we could have a test that fails when the code isn't there.
>
> We do - if you remove the "if (Elements)" a Clang test case will fail
> the assertions (& without the assertions, it would fail a later
> assertion (see PR16995) in a subset of cases (none covered by test
> cases)) - I could construct a test case that would fail even in an
> assert-free build, but it's bit more involved. Happy to do so if you
> think that's valuable.

Perhaps take the code from the PR and verifying all of the elements
are there at least. It won't reproduce the bug necessarily but could
be a good general test.

>> Any reason we don't want the same check here? I haven't gone through
>> the path we're taking to get here from the bug to check.
>
> Template parameters aren't lazily added like members are, so this
> isn't a concern - I could build the superset check into a function &
> use it in both for symmetry, but I don't see it as a sufficiently
> interesting risk to secure against.

Reasonable.

>
>> Also, comments :)
>
> There was a comment at the start & a textual description in the assert
> - are they insufficient or are there other aspects that you'd like
> more clarification on?
>

Just a block comment above the verification code on what you're
verifying. I.e. something along the lines of "Iterate through the two
lists of elements and ensure that we've got an identical or superset
of elements." Comments are easier to read than the code.

-eric