Typedef rewritten unnecessarily with extra space.

[kyleheadley]

typedef int *Pint;

Pint ofBeer(void) {
  return 0;
}

rewrites to

typedef _Ptr<int>  Pint;

Pint  ofBeer(void) {
  return 0;
}

Note the extra space before "Pint" in the typedef and after "Pint" in the function def.
Also, the function didn't need rewriting, which adds formatting diffs to untouched code.

[mattmccutchen-cci]

As of 3C revision 50e0a25, I now get the following output:

typedef _Ptr<int> Pint;

Pint  ofBeer(void) {
  return 0;
}

The typedef definition has been fixed, but the function return type hasn't.

I think the basic problem is that the code that rewrites return types is designed for written-out unchecked pointer types such as int *ofBeer(void). FunctionDeclReplacement::getReturnEnd calls FunctionDecl::getReturnTypeSourceRange, which returns the int *. Then we replace this range with a string that ends in a space to ensure that it doesn't run together with the subsequent identifier. The space is added in one of these two places:

checkedc-clang/clang/lib/3C/ConstraintVariables.cpp

Lines 988 to 989 in 50e0a25

	if (IsReturn && !ForItype)
	Ss << " ";

checkedc-clang/clang/lib/3C/ConstraintVariables.cpp

Line 770 in 50e0a25

QualTypedef += " ";

But in the example in this issue, FunctionDecl::getReturnTypeSourceRange returns Pint without the space, as it should. When we replace that with Pint , we get an extra space.

As you point out, one way to avoid the problem is to detect that the function doesn't need rewriting, and we may want that anyway in order to preserve other formatting. We could also fix the extra space directly, which would benefit similar cases in which we actually do need to rewrite the function, if any such cases exist. (We can make two separate issues for these two changes if it seems worth the trouble.)

For the latter change, I guess we could try to directly query whether an identifier starts immediately after the FunctionDecl::getReturnTypeSourceRange and only add a space if so. (The nearest precedent I can think of for something like this in 3C is in PointerVariableConstraint::emitArraySize.) Alternatively, a reasonable approximation might be to check whether the originally written return type was of a form that we know ends with an identifier character (e.g., a typedef), meaning that it must already have been followed by a space and we don't need to add one. If we're not sure, we might add an unnecessary space. Maybe typedefs are actually the only cases that trigger the problem and checking specifically for a typedef is a fine solution.