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

[Richard Smith via cfe-commits]

rsmith accepted this revision.
rsmith added a reviewer: rsmith.
rsmith added a comment.
This revision is now accepted and ready to land.

LGTM with a couple of minor tweaks, thanks!


================
Comment at: lib/Tooling/Core/QualTypeNames.cpp:400-401
@@ -395,8 +399,4 @@
                                                true /*FullyQualified*/);
 
-  // move the qualifiers on the outer type (avoid 'std::const string'!)
-  if (Prefix) {
-    PrefixQualifiers = QT.getLocalQualifiers();
-    QT = QualType(QT.getTypePtr(), 0);
-  }
+  QT = QualType(QT.getTypePtr(), 0);
 
----------------
Sure, building the elaborated type should still be conditional on there being one to build :)

`const X *` looks correct to me for your new example (I think with the old code we'd have got just `X *`).

================
Comment at: lib/Tooling/Core/QualTypeNames.cpp:401
@@ -396,7 +400,3 @@
 
-  // move the qualifiers on the outer type (avoid 'std::const string'!)
-  if (Prefix) {
-    PrefixQualifiers = QT.getLocalQualifiers();
-    QT = QualType(QT.getTypePtr(), 0);
-  }
+  QT = QualType(QT.getTypePtr(), 0);
 
----------------
Please move this up to immediately after line 389, so that we have a single point where we split `QT` into a type pointer and qualifiers.

================
Comment at: lib/Tooling/Core/QualTypeNames.cpp:408
@@ -407,3 +407,3 @@
     // We are asked to fully qualify and we have a Record Type (which
     // may pont to a template specialization) or Template
     // Specialization Type. We need to fully qualify their arguments.
----------------
Maybe pont -> point while you're here :)

================
Comment at: lib/Tooling/Core/QualTypeNames.cpp:412
@@ -412,3 +411,3 @@
     const Type *TypePtr = getFullyQualifiedTemplateType(Ctx, QT.getTypePtr());
-    QT = Ctx.getQualifiedType(TypePtr, Quals);
+    QT = Ctx.getQualifiedType(TypePtr, Qualifiers());
   }
----------------
`QT = QualType(TypePtr, 0);` would be fine here.


http://reviews.llvm.org/D20040