Reconstruct a parameter declaration if we can't get its original source

due to a macro.

Cleanup: It doesn't seem like we need the special case for typedefs in
tyToStr. Indeed, if the type is actually a TypedefType, I'd expect Clang
to print it as such.

Author: mattmccutchen-cci

clang/include/clang/3C/Utils.h

@@ -142,7 +142,12 @@ bool isVarArgType(const std::string &TypeName);
 bool isStructOrUnionType(clang::VarDecl *VD);
 
 // Helper method to print a Type in a way that can be represented in the source.
-std::string tyToStr(const clang::Type *T);
+// If Name is given, it is included as the variable name (which otherwise isn't
+// trivial to do with function pointers, etc.).
+std::string tyToStr(const clang::Type *T, const std::string &Name = "");
+
+// Same as tyToStr with a QualType.
+std::string qtyToStr(clang::QualType QT, const std::string &Name = "");
 
 // Get the end source location of the end of the provided function.
 clang::SourceLocation getFunctionDeclRParen(clang::FunctionDecl *FD,

clang/lib/3C/DeclRewriter.cpp

@@ -700,6 +700,10 @@ void FunctionDeclBuilder::buildItypeDecl(PVConstraint *Defn,
   return;
 }
 
+// Note: For a parameter, Type + IType will give the full declaration (including
+// the name) but the breakdown between Type and IType is not guaranteed. For a
+// return, Type will be what goes before the name and IType will be what goes
+// after the parentheses.
 void
 FunctionDeclBuilder::buildDeclVar(PVConstraint *IntCV, PVConstraint *ExtCV,
                                   DeclaratorDecl *Decl, std::string &Type,
@@ -725,19 +729,26 @@ FunctionDeclBuilder::buildDeclVar(PVConstraint *IntCV, PVConstraint *ExtCV,
   // For parameter variables, we try to extract the declaration from the source
   // code. This preserves macros and other formatting. This isn't possible for
   // return variables because the itype on returns is located after the
-  // parameter list. Sometimes the source range for a parameter declaration is
-  // not valid, for example if a function prototype is declared using a typedef.
-  // For these cases, we just fall back to reconstructing the declaration from
-  // the PVConstraint.
-  SourceRange Range = Decl->getSourceRange();
-  if (isa<ParmVarDecl>(Decl) && Range.isValid()) {
-    Type = getSourceText(Range, *Context);
-    IType = "";
+  // parameter list. Sometimes we cannot get the original source for a parameter
+  // declaration, for example if a function prototype is declared using a
+  // typedef or the parameter declaration is inside a macro. For these cases, we
+  // just fall back to reconstructing the declaration from the PVConstraint.
+  ParmVarDecl *PVD = dyn_cast<ParmVarDecl>(Decl);
+  if (PVD) {
+    Type = getSourceText(PVD->getSourceRange(), *Context);
+    if (!Type.empty()) {
+      // Great, we got the original source including any itype and bounds.
+      IType = "";
+      return;
+    }
+    // Otherwise, reconstruct the name and type, and reuse the code below for
+    // the itype and bounds.
+    Type = qtyToStr(PVD->getOriginalType(), PVD->getNameAsString());
   } else {
     Type = ExtCV->getOriginalTy() + " ";
-    IType = getExistingIType(ExtCV);
-    IType += ABRewriter.getBoundsString(ExtCV, Decl, !IType.empty());
   }
+  IType = getExistingIType(ExtCV);
+  IType += ABRewriter.getBoundsString(ExtCV, Decl, !IType.empty());
 }
 
 std::string FunctionDeclBuilder::getExistingIType(ConstraintVariable *DeclC) {

clang/lib/3C/Utils.cpp

@@ -224,15 +224,14 @@ bool isStructOrUnionType(clang::VarDecl *VD) {
          VD->getType().getTypePtr()->isUnionType();
 }
 
-std::string tyToStr(const clang::Type *T) {
-  if (auto TDT = dyn_cast<TypedefType>(T)) {
-    auto D = TDT->getDecl();
-    std::string s = D->getNameAsString();
-    return s;
-  } else {
-    QualType QT(T, 0);
-    return QT.getAsString();
-  }
+std::string qtyToStr(clang::QualType QT, const std::string &Name) {
+  std::string S = Name;
+  QT.getAsStringInternal(S, LangOptions());
+  return S;
+}
+
+std::string tyToStr(const clang::Type *T, const std::string &Name) {
+  return qtyToStr(QualType(T, 0), Name);
 }
 
 Expr *removeAuxillaryCasts(Expr *E) {

clang/test/3C/params_in_macro.c

@@ -0,0 +1,24 @@
+// Test that if FunctionDeclBuilder::buildDeclVar cannot get the original source
+// for a parameter declaration due to a macro, it reconstructs the declaration
+// (including the name) instead of leaving a blank.
+
+// RUN: 3c -base-dir=%S %s -- | FileCheck -match-full-lines %s
+// RUN: 3c -base-dir=%S %s -- | %clang -c -fcheckedc-extension -x c -o /dev/null -
+
+// 3C is not idempotent on this file because the first pass constrains the
+// pointers in the macros to wild but then inlines the macros, so the second
+// pass will make those pointers checked. So don't try to test idempotence.
+
+typedef double mydouble;
+
+// TODO: FunctionDeclBuilder::buildDeclVar should be able to handle an itype
+// here, but currently the PointerConstraintVariable constructor asserts when it
+// fails to retrieve the original source of the itype declaration.
+#define parms1 volatile mydouble d, void (*f)(void)
+#define parms2 int *const y : count(7), _Ptr<char> z
+
+void test(parms1, int *x, parms2) {}
+// CHECK: void test(volatile mydouble d, void (*f)(void), _Ptr<int> x, int *const y : count(7), _Ptr<char> z) {}
+
+// Before the bug fix, we got:
+// void test(, , _Ptr<int> x, , ) {}