[PATCH] D116635: Add warning to detect when calls passing arguments are made to functions without prototypes.

[Dan Liew via Phabricator via cfe-commits]

delcypher added inline comments.


================
Comment at: clang/test/Sema/warn-calls-without-prototype.c:39
+  return a + b +c;
+}
+
----------------
delcypher wrote:
> @NoQ Any ideas about this?  It seems kind of weird that when merging `not_a_prototype3` prototype with the K&R style definition of `not_a_prototype3` that the resulting FunctionDecl we see at the call site in `call_to_function_without_prototype3` is marked as not having a prototype.
> 
> If I flip the order (see `not_a_prototype6`) then the merged declaration is marked as having a prototype.
> 
> I'm not sure if this is a bug in `Sema::MergeFunctionDecl` or if this just a peculiarity of K&R style function definitions.
I suspect the problem might be here in `Sema::MergeFunctionDecl`.

```lang=c++
   // C: Function types need to be compatible, not identical. This handles
  // duplicate function decls like "void f(int); void f(enum X);" properly.
  if (!getLangOpts().CPlusPlus &&
      Context.typesAreCompatible(OldQType, NewQType)) {
    const FunctionType *OldFuncType = OldQType->getAs<FunctionType>();
    const FunctionType *NewFuncType = NewQType->getAs<FunctionType>();
    const FunctionProtoType *OldProto = nullptr;
    if (MergeTypeWithOld && isa<FunctionNoProtoType>(NewFuncType) &&
        (OldProto = dyn_cast<FunctionProtoType>(OldFuncType))) {
      // The old declaration provided a function prototype, but the
      // new declaration does not. Merge in the prototype.
```

` isa<FunctionNoProtoType>(NewFuncType)` is false in this particular case, however `New` doesn't have a prototype (i.e. `New->hasPrototype()` is false). One fix might be to replace `isa<FunctionNoProtoType>(NewFuncType)` with 

```
(isa<FunctionNoProtoType>(NewFuncType) || !New->hasPrototype())
```

However, I don't really know this code well enough to know if that's the right fix.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116635