[PATCH] D104619: [clang] [WIP] Respect PrintingPolicy::FullyQualifiedName when printing a template-id

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

In D104619#2831956 <https://reviews.llvm.org/D104619#2831956>, @nridge wrote:

> Thanks for having a look!
>
> In D104619#2831953 <https://reviews.llvm.org/D104619#2831953>, @dblaikie wrote:
>
>> This'll need a test case
>
> Definitely. Do you have a suggestion for what test suite that should go into? I had a quick look but couldn't find anything that obviously exercised `TypePrinter`.

One way you could find some places this might be tested would be to introduce a deliberate break in the code somehow - like changing this parameter to be "true" instead of false - and then running the tests to see what fails. But I think the problem is likely that the tests all want the existing behavior/never tweak the Policy in this context.

So likely what you'd need would be a unit test of some kind (check around in clang/unittests to see if there are other AST pretty printer tests, for instance).

>> & does the change pass all existing tests?
>
> I mainly pushed the patch to Phabricator in this WIP form because I was hoping that would trigger some sort of CI run that would tell me that :) (I tried running `ninja check-clang` locally, but that was looking to take a very long time (like 2+ hours) to run locally.)

Hmm - what sort of machine config do you have? What build configuration were you building/running with?

> It's not clear to me if that actually happened? I see something about "pre-merge checks" in the revision metadata (and they're green), but it's not clear to me what test suites it actually ran.

Yeah, it looks like some tests ran: https://buildkite.com/llvm-project/premerge-checks/builds/44109 (if you follow from the "Build 157317: pre-merge checks" under the patch description, then copy/paste the URL from somewhere in there you'll get to that build info


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D104619/new/

https://reviews.llvm.org/D104619