[PATCH] D20040: Treat qualifiers on elaborated types for qualtypenames appropriately.

[Richard Smith via cfe-commits]

rsmith added a subscriber: rsmith.

================
Comment at: lib/Tooling/Core/QualTypeNames.cpp:386
@@ -385,2 +385,3 @@
   NestedNameSpecifier *Prefix = nullptr;
-  Qualifiers PrefixQualifiers;
+  // Local qualifiers are attached to the Qualtype outside of the
+  // elaborated type.  Retrieve them before descending into the
----------------
Qualtype -> QualType

================
Comment at: lib/Tooling/Core/QualTypeNames.cpp:392
@@ -388,3 +391,3 @@
   if (const auto *ETypeInput = dyn_cast<ElaboratedType>(QT.getTypePtr())) {
     QT = ETypeInput->getNamedType();
     Keyword = ETypeInput->getKeyword();
----------------
Maybe add an assert that `QT` is unqualified here?

================
Comment at: lib/Tooling/Core/QualTypeNames.cpp:401-403
@@ -397,6 +400,5 @@
   // move the qualifiers on the outer type (avoid 'std::const string'!)
-  if (Prefix) {
-    PrefixQualifiers = QT.getLocalQualifiers();
+  if (Prefix || Keyword != ETK_None) {
     QT = QualType(QT.getTypePtr(), 0);
   }
 
----------------
I find the way this code ensures that we preserve the qualifiers to be a little subtle. It's not obvious to me that this does the right thing for a case like

    struct X;
    void f(const ::X x) {}

... where we have an `ElaboratedType` with no keyword, and for which we will generate an empty `Prefix` -- it looks like we would lose the `const` on line 392 and never add it back.

Can you remove and re-add the qualifiers unconditionally? (That is, move this removal of qualifiers from `QT` to after line 389, and move line 419 outside the `if`.) I think that'll make the logic clearer.

================
Comment at: lib/Tooling/Core/QualTypeNames.cpp:413-415
@@ -410,5 +412,5 @@
 
     Qualifiers Quals = QT.getLocalQualifiers();
     const Type *TypePtr = getFullyQualifiedTemplateType(Ctx, QT.getTypePtr());
     QT = Ctx.getQualifiedType(TypePtr, Quals);
   }
----------------
With the change suggested above, you should be able to delete the handling of qualifiers here.


http://reviews.llvm.org/D20040