Michael137 wrote:

> * This could probably get rid of the additional bool flags, and rely on 
> PrintingPolicy entirely.
> * Instead of the `printAnonymousTagDecl` helper, we could probably implement 
> a `print` method for TagDecls, which takes care of the named case, and for 
> the unnamed case, takes care of the EnumDecl and RecordDecl specific 
> behavior, including the typedef to unnamed class.

This is mostly implemented now in the latest patchset. My only reservation 
about moving the printing logic into `TagDecl::printName` is that we no longer 
are able to easily determine whether to print the tag keyword. Previously we've 
done this in: 
https://github.com/llvm/llvm-project/blob/762a171b3d6f6eb64dc5a844fcb25caa4ece7d00/clang/lib/AST/TypePrinter.cpp#L1523-L1529

But now we have to rely on the `PrintingPolicy` telling us this, putting the 
burden on the callers (specifically callers of `QualType::print`). Maybe that's 
an OK trade-off? You can see this in the `clangd/unittests/HoverTests.cpp` 
changes in the latest revision. We now don't omit the tag keyword in the name 
itself. For the `Hover` case specifically we could fix this by setting the new 
`SuppressTagKeywordInAnonNames` policy here: 
https://github.com/llvm/llvm-project/blob/762a171b3d6f6eb64dc5a844fcb25caa4ece7d00/clang-tools-extra/clangd/Hover.cpp#L175-L183

But there's probably other places where this needs to be adjusted. Wdyt? Is 
that a reasonable trade-off?

https://github.com/llvm/llvm-project/pull/169445
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to