aaron.ballman added reviewers: dblaikie, sammccall. aaron.ballman added subscribers: sammccall, dblaikie. aaron.ballman added a comment.
In D149677#4350337 <https://reviews.llvm.org/D149677#4350337>, @li.zhe.hua wrote: > In D149677#4349523 <https://reviews.llvm.org/D149677#4349523>, @aaron.ballman > wrote: > >> In D149677#4320178 <https://reviews.llvm.org/D149677#4320178>, @li.zhe.hua >> wrote: >> >>> Some additional context: I'm working on a refactoring tool to, in part, >>> deprecate an alias by replacing it with its definition. That functionality >>> by itself could be provided through something like a clang-tidy, so I say >>> "initially". >> >> We typically don't add new printing policies in tree without some need for >> them in the project itself (otherwise this becomes a maintenance problem), >> and I'm not certain I see a strong need for this to live in Clang. I think >> it might be best to wait until you're closer to having a clang-tidy check >> that would use this functionality before we continue with this patch. WDYT? > > I believe there is value for having this option, as without it, the > `PrintingCallbacks::isScopeVisible` callback is effectively useless in > practice. It won't be triggered in non-canonical types because the > `ElaboratedType` represents a fixed qualification on the name. > > Note: there may be an argument for folding this functionality directly into > `FullyQualifiedName` (which currently affects template-ids despite the > comment on it suggesting it only is for function names). If the template name > was elaborated, `FullyQualifiedName` does nothing, which seems > inconsistent/incorrect. Adding @dblaikie as he might have some opinions here. I'm not seeing how the changes you've got here interact with `isScopeVisible()`; However, I also note that the only use of `isScopeVisible()` in tree is in clangd, so also adding @sammccall to see if they're hitting issues there or not. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D149677/new/ https://reviews.llvm.org/D149677 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits