aaron.ballman added a comment. In D134813#3834613 <https://reviews.llvm.org/D134813#3834613>, @sammccall wrote:
> In D134813#3834540 <https://reviews.llvm.org/D134813#3834540>, @aaron.ballman > wrote: > >> In D134813#3834496 <https://reviews.llvm.org/D134813#3834496>, @sammccall >> wrote: >> >>> Sorry, Monday was a holiday here... >> >> No worries, I hope you had a good holiday! > > Thanks :-) > >>> I don't think unconditionally including the filename in >>> printQualifiedName() is great for tools, it can be unreasonably long and is >>> generally just noise when shown in the context of that file. I'm surprised >>> that USR generation + that clangd test are the only things that broke! Poor >>> test coverage in the tools, I think :-( >>> If the intent is to change this for diagnostics only, can it be behind a >>> PrintingPolicy flag? >> >> Diagnostics are largely the intent for this and I could probably put a >> printing policy flag, but I'm not yet convinced that printing nothing is >> actually better for tools in general. > > Agreed - I think in my perfect world we'd switch between `(unnammed struct)` > and `(unnamed struct at ...)`. > ... wait, we already have `PrintingPolicy::AnonymousTagLocations`, looks like > that does what I want. I've set this for clangd in > e212a4f838f17e2d37b1d572d8c1b49c50d7fe17 > <https://reviews.llvm.org/rGe212a4f838f17e2d37b1d572d8c1b49c50d7fe17>, so > this patch should be fine with just updating the string in the test to > `(unnamed class)`. Hehe, I was getting a start on your idea when I discovered that as well. :-D >> I think it really boils down to whether the name is user-facing or not. >> e.g., for USRs, it seems like we don't want to print this sort of thing, >> which is fine because users don't stare at those. But tools like clang-query >> output various names of things based on user queries, and it seems like it's >> less useful for that output to print nothing for the name. > > Yeah, I can't see a case where silently printing *nothing* is clearly what we > want. > Even USRs mostly don't do that: they try to print the name, detect nothing > got printed, and print something else instead! > >> That said, it still sounds like we should have a printing policy for it, but >> what should the default be? (I lean towards "print more information instead >> of less".) > > Given that we have a way to suppress the filename, I don't think we should > add another printing policy (until we see a reason to). > Changing USR generation to not rely on this detail seems easier, I can take a > stab at this. Thank you, I think that's a good idea. I'll rebase on top of those changes. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134813/new/ https://reviews.llvm.org/D134813 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
