Don't add void prototypes on functions that would otherwise not need to be rewritten (#688)

A function that does not otherwise need to be rewritten would sometimes be
rewritten in order to replace a missing prototype with (void). This can trigger
a rewriting error if the function is defined by a macro or in an unwritable
file.

After these changes, 3C still adds a prototype for functions with checked or
itype return types, so int `*foo() { return 0; }` will still convert to
`_Ptr<int> foo(void) { return 0; }` as is required by Checked C.

Checked C also requires that a functions has a prototype for it to be called
from inside a checked scope, but this is not handled by 3C; see issue
#382.  The status of this issue has not
changed.

Author: john-h-kastner

clang/lib/3C/DeclRewriter.cpp

@@ -567,7 +567,8 @@ bool FunctionDeclBuilder::VisitFunctionDecl(FunctionDecl *FD) {
   bool RewriteGeneric = false;
 
   bool DeclIsTypedef = false;
-  if (TypeSourceInfo *TS = FD->getTypeSourceInfo()) {
+  TypeSourceInfo *TS = FD->getTypeSourceInfo();
+  if (TS != nullptr) {
     // This still could possibly be a typedef type if TS was NULL.
     // TypeSourceInfo is null for implicit function declarations, so if a
     // implicit declaration uses a typedef, it will be missed. That's fine
@@ -623,12 +624,10 @@ bool FunctionDeclBuilder::VisitFunctionDecl(FunctionDecl *FD) {
         RewriteParams = true;
     }
   } else {
-    // No params and no param source: make explicit
+    // Functions in CheckedC need prototypes, so replace empty parameter lists
+    // with an explict (void). This updates the parameter list; the rewrite flag
+    // will be set once it is known if the return needs to be rewritten.
     ParmStrs.push_back("void");
-    QualType ReturnTy = FD->getReturnType();
-    QualType Ty = FD->getTypeSourceInfo()->getType();
-    if (!Ty->isFunctionProtoType() && ReturnTy->isPointerType())
-      RewriteParams = true;
   }
 
   // Get rewritten return variable.
@@ -661,6 +660,15 @@ bool FunctionDeclBuilder::VisitFunctionDecl(FunctionDecl *FD) {
     RewriteParams = true;
   }
 
+  // If this function was declared without a prototype, then we must add one
+  // to be able to give it a checked return type. This was done by adding "void"
+  // to the parameter list above. Here we indicate the parameter list should be
+  // rewritten to include "void" only if the return is already being rewritten.
+  // This avoids unnecessarily adding void to empty parameter lists on unchecked
+  // functions.
+  if (TS && !TS->getType()->isFunctionProtoType() && RewriteReturn)
+    RewriteParams = true;
+
   // If the function is declared using a typedef for the function type, then we
   // need to rewrite parameters and the return if either would have been
   // rewritten. What this does is expand the typedef to the full function type

clang/test/3C/dont_add_prototype.c

@@ -0,0 +1,34 @@
+// RUN: rm -rf %t*
+// RUN: 3c -base-dir=%S -alltypes -addcr %s -- | FileCheck -match-full-lines -check-prefixes="CHECK_ALL","CHECK" %s
+// RUN: 3c -base-dir=%S -addcr %s -- | FileCheck -match-full-lines -check-prefixes="CHECK_NOALL","CHECK" %s
+// RUN: 3c -base-dir=%S -alltypes -addcr %s -- | %clang -c -fcheckedc-extension -x c -o /dev/null -
+// RUN: 3c -base-dir=%S -alltypes -output-dir=%t.checked %s --
+// RUN: 3c -base-dir=%t.checked -alltypes %t.checked/dont_add_prototype.c -- | diff %t.checked/dont_add_prototype.c -
+
+// Don't add a void prototype on functions if we don't need to. This can
+// potentially cause rewriting errors when attempting to rewrite functions
+// defined outside the basedir or defined by a macro.
+
+// This function does not need to be rewritten. Its return type is unchecked
+// after solving. It should not be rewritten to use a void prototype in order
+// to avoid any potential rewritting issues.
+void *test0() { return 1; }
+//CHECK: void *test0() { return 1; }
+
+// Trying to add a prototype in these examples caused a rewriting error
+// because the functions are defined by macros.
+
+#define test_macro0 int *test_macro0()
+test_macro0 {
+//CHECK: test_macro0 {
+  return 0;
+}
+
+#define test_macro1 test_macro1()
+int *test_macro1 {
+//CHECK: int *test_macro1 {
+  return 0;
+}
+
+// Force conversion output.
+int *a;

clang/test/3C/generalize.c

@@ -22,10 +22,10 @@ void viewer_update(void *i) {
 void *getNull() { return 0; }
 // CHECK: _For_any(T) _Ptr<T> getNull(void) { return 0; }
 void *getOne() { return 1; }
-// CHECK: void *getOne(void) { return 1; }
+// CHECK: void *getOne() { return 1; }
 extern void *glob = 0;
 void *getGlobal() { return glob; }
-// CHECK: void *getGlobal(void) { return glob; }
+// CHECK: void *getGlobal() { return glob; }
 
 // check type parameters
 void call_from_fn() {