[PATCH] D36368: Fix type printing of array template args

[John McCall via Phabricator via llvm-commits]

rjmccall added a comment.

Sorry, this got lost.



================
Comment at: clang/include/clang/AST/Type.h:1895
+  /// Returns the inner most Pointer Type node.
+  const PointerType *getInnerMostPointerType() const;
+
----------------
I don't think this makes any sense to add as a general routine.  If we do add it, though, it should be `getInnermostPointerType()`; "innermost" is a single word.


================
Comment at: clang/lib/AST/TemplateBase.cpp:380
+      }
+    }
+    if (!getParamTypeForDecl()->isReferenceType() && needsRef)
----------------
Please make a helper function for deciding whether an ampersand is required.

There's a lot more going on in this code than needs to be there; I don't know what this string comparison is supposed to accomplish, for example.  It's okay to assume that the argument is well-formed for the parameter, which means you really do just need to compare pointer depths, like:

```
static unsigned getPointerDepth(QualType type) {
  unsigned count;
  while (auto ptrType = type->getAs<PointerType>()) {
    count++;
    type = ptrType->getPointeeType();
  }
  return count;
}

static bool needsAmpersandOnTemplateArg(QualType paramType, QualType argType) {
  if (paramType->isReferenceType()) return false;
  if (argType->isArrayType()) {
    // Decide whether we've decayed the array (to get a pointer to the element type)
    // or taken its address (to get a pointer to the array type).  We have to ignore
    // qualifiers here at arbitrary levels because of qualification conversions.
    return (getPointerDepth(paramType) >
            getPointerDepth(QualType(argType->getArrayElementTypeNoTypeQual(), 0)));
  }
  return true;
}
```


================
Comment at: clang/test/SemaTemplate/temp_arg_nontype_cxx11.cpp:89
+  Holumn<&kNull, double>().ls();  // expected-error {{<&kNull,}}
+  Jolumn<kZero, double>().ls();   // expected-error {{<kZero,}}
+}
----------------
Please add both positive and negative tests (i.e. both array decays and otherwise) for each of these.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D36368/new/

https://reviews.llvm.org/D36368