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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits