mkString fixes that were previously part of the multi-decl overhaul.

Back when the multi-decl overhaul
(#657) switched
all unchanged multi-decl members from Decl::print to mkString, these
fixes were needed to prevent 3C's output from getting worse on some
unchanged multi-decl members in the regression tests. The fixes are no
longer needed for that reason, but Mike still asked me to submit them.
They do benefit some more complex cases that occur in the regression
tests, so the updates to the expected output of those tests provide test
coverage for the fixes.

Author: mattmccutchen-cci

clang/lib/3C/ConstraintVariables.cpp

@@ -833,10 +833,31 @@ PointerVariableConstraint::mkString(Constraints &CS,
     if (!ForItype && InferredGenericIndex == -1 && isVoidPtr())
       K = Atom::A_Wild;
 
+    // In a case like `_Ptr<T> g[1]`, we need to push the ` g` onto EndStrs
+    // before we can push the `>` and start drilling into T.
+    //
+    // Exception: In a case like `int *g[1]`, we don't want an extra space after
+    // the `*` but we don't yet know whether we have a `*`, so we skip emitting
+    // the name. We have to also skip the addArrayAnnotations because it would
+    // cause the array annotations to be emitted before the name.
+    //
+    // Exception to the exception: In a case like `void (*fs[2])()`, we still
+    // want to avoid emitting the name (which would add an extra space after the
+    // `*`), but we _do_ need to call addArrayAnnotations so the array
+    // annotations end up with the variable name rather than after the function
+    // parameter list. The function pointer code knows to emit the name before
+    // EndStrs.
+    //
+    // This passes the current regression tests but feels very ad-hoc.
+    // REVIEW: Help me redesign this code!
     if (PrevArr && ArrSizes.at(TypeIdx).first != O_SizedArray && !EmittedName) {
-      EmittedName = true;
-      addArrayAnnotations(ConstArrs, EndStrs);
-      EndStrs.push_front(" " + UseName);
+      if (K != Atom::A_Wild || FV != nullptr) {
+        addArrayAnnotations(ConstArrs, EndStrs);
+      }
+      if (K != Atom::A_Wild) {
+        EmittedName = true;
+        EndStrs.push_front(" " + UseName);
+      }
     }
     PrevArr = ArrSizes.at(TypeIdx).first == O_SizedArray;
 
@@ -926,6 +947,8 @@ PointerVariableConstraint::mkString(Constraints &CS,
   // the the stack array type.
   if (PrevArr && !EmittedName && AllArrays) {
     EmittedName = true;
+    // Note: If the whole type is an array, we can't have a "*", so we don't
+    // need to worry about an extra space.
     EndStrs.push_front(" " + UseName);
   }
 

clang/test/3C/compound_literal.c

@@ -67,7 +67,7 @@ void lists() {
 
   int *d[2] = (int *[2]){&x, (int *)1};
   //CHECK_NOALL: int *d[2] = (int *[2]){&x, (int *)1};
-  //CHECK_ALL: int * d _Checked[2] = (int * _Checked[2]){&x, (int *)1};
+  //CHECK_ALL: int *d _Checked[2] = (int * _Checked[2]){&x, (int *)1};
   int *d0 = d[0];
   //CHECK: int *d0 = d[0];
 

clang/test/3C/fptr_array.c

@@ -9,15 +9,15 @@
 
 void (*fs[2])(int *);
 void (*f)(int *);
-//CHECK_NOALL: void (* fs[2])(_Ptr<int>) = {((void *)0)};
+//CHECK_NOALL: void (*fs[2])(_Ptr<int>) = {((void *)0)};
 //CHECK_NOALL: void (*f)(_Ptr<int>) = ((void *)0);
 //CHECK_ALL: _Ptr<void (_Ptr<int>)> fs _Checked[2] = {((void *)0)};
 //CHECK_ALL: _Ptr<void (_Ptr<int>)> f = ((void *)0);
 
 void (*gs[2])(int *);
 void g_impl(int *x) { x = 1; }
 void (*g)(int *) = g_impl;
-//CHECK_NOALL: void (* gs[2])(int * : itype(_Ptr<int>)) = {((void *)0)};
+//CHECK_NOALL: void (*gs[2])(int * : itype(_Ptr<int>)) = {((void *)0)};
 //CHECK_NOALL: void g_impl(int *x : itype(_Ptr<int>)) { x = 1; }
 //CHECK_NOALL: void (*g)(int * : itype(_Ptr<int>)) = g_impl;
 
@@ -30,21 +30,21 @@ void (*h)(void *);
 
 int *(*is[2])(void);
 int *(*i)(void);
-//CHECK_NOALL: _Ptr<int> (* is[2])(void) = {((void *)0)};
+//CHECK_NOALL: _Ptr<int> (*is[2])(void) = {((void *)0)};
 //CHECK_NOALL: _Ptr<int> (*i)(void) = ((void *)0);
 //CHECK_ALL: _Ptr<_Ptr<int> (void)> is _Checked[2] = {((void *)0)};
 //CHECK_ALL: _Ptr<_Ptr<int> (void)> i = ((void *)0);
 
 int *(**js[2])(void);
 int *(**j)(void);
-//CHECK_NOALL: _Ptr<int> (** js[2])(void) = {((void *)0)};
+//CHECK_NOALL: _Ptr<int> (**js[2])(void) = {((void *)0)};
 //CHECK_NOALL: _Ptr<int> (**j)(void) = ((void *)0);
 //CHECK_ALL: _Ptr<_Ptr<_Ptr<int> (void)>> js _Checked[2] = {((void *)0)};
 //CHECK_ALL: _Ptr<_Ptr<_Ptr<int> (void)>> j = ((void *)0);
 
 int *(*ks[2][2])(void);
 int *(*k)(void);
-//CHECK_NOALL: _Ptr<int> (* ks[2][2])(void) = {((void *)0)};
+//CHECK_NOALL: _Ptr<int> (*ks[2][2])(void) = {((void *)0)};
 //CHECK_NOALL: _Ptr<int> (*k)(void) = ((void *)0);
 //CHECK_ALL: _Ptr<_Ptr<int> (void)> ks _Checked[2] _Checked[2] = {((void *)0)};
 //CHECK_ALL: _Ptr<_Ptr<int> (void)> k = ((void *)0);

clang/test/3C/ptr_array.c

@@ -28,7 +28,7 @@ void test1(int *a) {
 
   int *b[1] = {a};
   //CHECK_NOALL: int *b[1] = {a};
-  //CHECK_ALL: int * b _Checked[1] = {a};
+  //CHECK_ALL: int *b _Checked[1] = {a};
 }
 
 /* Example from from the issue */
@@ -40,7 +40,7 @@ int *foo() {
   int z = 3;
   int *ptrs[4] = {&x, &y, &z, (int *)5};
   //CHECK_NOALL: int *ptrs[4] = {&x, &y, &z, (int *)5};
-  //CHECK_ALL: int * ptrs _Checked[4] = {&x, &y, &z, (int *)5};
+  //CHECK_ALL: int *ptrs _Checked[4] = {&x, &y, &z, (int *)5};
   int *ret;
   //CHECK: int *ret;
   for (int i = 0; i < 4; i++) {