Fix PR13444 - wrong mangling of "const char * const *" and friends

Hi John,

Can you please review this patch?
This fixes http://llvm.org/PR13444 as well as a large part of
http://llvm.org/PR13182 plus adds quite a few tests.

With this patch plus the pending PR13455 patch I don't get link-time
problems when building SPEC CPU2006 tests anymore!
(I'm only testing the files that are compileable with Clang++ though,
e.g. no RTTI users etc)

FTR, the SplitQualType / isa<ArrayType> part was mostly taken from
ItaniumMangle.cpp

Thanks in advance,
Timur Iskhodzhanov,
Google Russia

bug_13444.patch (13.2 KB)

Nice patch, LGTM.

I really like the clean up of all the pointer qualifier mangling into manglePointerQualifiers, makes the code much simpler to follow.

I wonder if this could be made a bit more clear, a la something like this:

bool HasConst = Quals.hasConst();
bool HasVolatile = Quals.hasVolatile();

if (hasVolatile && hasConst)
  Out << 'S';
else if (hasConst)
  Out << 'Q';
else if (hasVolatile)
  Out << 'R';
else
  Out << 'P';

As it stands, I had to think about the logic due to the nesting.
Other than that, it LGTM. Thanks!

~Aaron

Can you please review this patch?
This fixes http://llvm.org/PR13444 as well as a large part of
http://llvm.org/PR13182 plus adds quite a few tests.

+void MicrosoftCXXNameMangler::manglePointerQualifiers(Qualifiers Quals) {
+ // <pointer-cvr-qualifiers> ::= P # no qualifiers
+ // ::= Q # const
+ // ::= R # volatile
+ // ::= S # const volatile
+ if (!Quals.hasVolatile()) {
+ if (!Quals.hasConst()) {
+ Out << 'P';
+ } else {
+ Out << 'Q';
+ }
+ } else {
+ if (!Quals.hasConst()) {
+ Out << 'R';
+ } else {
+ Out << 'S';
+ }
+ }
+}

I wonder if this could be made a bit more clear, a la something like this:

bool HasConst = Quals.hasConst();
bool HasVolatile = Quals.hasVolatile();

if (hasVolatile && hasConst)
  Out << 'S';
else if (hasConst)
  Out << 'Q';
else if (hasVolatile)
  Out << 'R';
else
  Out << 'P';

As it stands, I had to think about the logic due to the nesting.

Actually what you're suggesting is exactly how I initially wrote it :slight_smile:
But then I saw MicrosoftCXXNameMangler::mangleQualifiers() and decided
to take that approach.
Let me fix both functions then...

Other than that, it LGTM. Thanks!

I suppose your LGTM is enough to commit.

r163110, thanks!

and r163111 for the new test file - blame svn for that :slight_smile: