saugustine added a comment.
Thanks for the reviews. I believe I have addressed all issues. Take another
look when you get the chance.
================
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);
}
----------------
rsmith wrote:
> 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.
We can remove and put them back unconditionally, but we still need to condition
getting a new elaborated type, because an assertion prevents creating an
elaborated type without a prefix of an empty keyword.
I'm not sure what you expect on the new example, but I have added it as a test
case (as a pointer because the struct is incomplete). The result we produce in
this case is "const X *", which I think is correct.
================
Comment at: unittests/Tooling/QualTypeNamesTest.cpp:91
@@ -90,2 +90,3 @@
Visitor.ExpectedQualTypeNames["AliasTypeVal"] = "A::B::C::InnerAlias<int>";
+ Visitor.ExpectedQualTypeNames["CheckM"] = "const A::B::Class0 *";
Visitor.runOver(
----------------
dblaikie wrote:
> What does this produce without your change? (what's the change causing to
> happen?)
Without the change, we get "A::B::Class0 *" with no "const".
http://reviews.llvm.org/D20040
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
