[PATCH] D80800: Add an option to fully qualify classes and structs.

Sam McCall via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri May 29 07:34:27 PDT 2020


sammccall added a comment.

This should really have a test - NamedDeclPrinterTest.cpp seems like the right place.



================
Comment at: clang/lib/AST/TypePrinter.cpp:326
+  if (Policy.FullyQualifiedName && Policy.GlobalScopeQualifiedName &&
+      T->isStructureOrClassType()) {
+    OS << "::";
----------------
structure-or-class-type isn't quite the right check:
 - enums and typedefs for instance should also be qualified.
 - you're looking through sugar to the underlying type, which may not be what you want
 
It's going to be hard to get this right from here... I suspect it's better to fix where `FullyQualifiedName` is checked in DeclPrinter.
This ultimately calls through to NamedDecl::printNestedNameSpecifier, which is probably the right place to respect this option.
(I don't totally understand what's meant to happen if SuppressScope is false but FullyQualiifedName is also false, that calls through to NestedNameSpecifier::print which you may want to also fix, or not)


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

https://reviews.llvm.org/D80800





More information about the llvm-commits mailing list