Clang's -Wmissing-prototypes isn't helpful for explicitly extern "C" function definitions.

[chandlerc]

I really like -Wmissing-prototypes for a lot of stuff, but it is firing on one pattern where it seems pretty unhelpful:

toolchain/parse/parse_fuzzer.cpp:18:16: error: no previous prototype for function 'LLVMFuzzerTestOneInput' [-Werror,-Wmissing-prototypes]
   18 | extern "C" int LLVMFuzzerTestOneInput(const unsigned char* data,
      |                ^
toolchain/parse/parse_fuzzer.cpp:18:12: note: declare 'static' if the function is not intended to be used outside of this translation unit
   18 | extern "C" int LLVMFuzzerTestOneInput(const unsigned char* data,
      |            ^
      |            static

The code in question is following LibFuzzer's documented guidance to define the fuzzer entry point:

extern "C" int LLVMFuzzerTestOneInput(const unsigned char* data,
                                      std::size_t size) {

The warning is technically correct that this definition has no prior prototype. However, by making it extern "C" there is a very clear signal that this is a known C ABI external entry point that we need to define, but may not have a header to declare for us. And suggesting to add static when the function is declared explicitly extern "C" seems pretty confusing.

Ideally I'd like to just disable this warning for extern "C" functions as it seems unlikely to be correct. If folks are specifically interested in retaining that, I'd love a flag to control that narrow behavior.

[chandlerc]

Another interesting case here are functions defined with __attribute__((weak)) (or equivalent) which seems specifically to be cases where a function might be defined legitimately without a prototype.

[shafik]

CC @AaronBallman

[firewave]

See also #55018.

[chandlerc]

See also #55018.

Thanks for connecting the issues!

I think the thing I disagree with is that there's nothing to do here. I think it would be good to restrict this warning so it doesn't fire for extern "C" functions (and maaaybe functions with weak linkage). If there are important use cases for warning on those, I'd suggest pulling that out into another category, -Wmissing-extern-C-prototypes maybe? And maybe this would even allow the warning to go inte -Wextra or -Wall.

I'm not using -Weverything, this warning found tons of issues with code I work with, and so it seems very high value. It seems worth working to carve out the cases with false positives and shift those to other flags/groups that can be disabled or enabled selectively.

[EugeneZelenko]

extern "C" does not describe API completely, so there is no guarantee that interface would not be broken if arguments will be changed on library side. So warning is completely correct in such situations.

[dwblaikie]

It looks like libFuzzer specifically does not want to vend a header for this function - the contract that libFuzzer and other fuzzers are using is that the library should expose a function that matches that signature, written in documentation only. ( https://llvm.org/docs/LibFuzzer.html - "Note that this fuzz target does not depend on libFuzzer in any way and so it is possible and even desirable to use it with other fuzzing engines")

This doesn't seem like a particularly uncommon use case - extern "C" APIs intended to be used from some other language that doesn't know about C, doesn't have C headers, and just says "have a symbol that matches this name and we'll pass it these arguments".

But, yes, many C APIs of course do correspond with C headers and are implemented in C++ files, so I'm not sure turning the warning off entirely in these cases would be ideal.

Having it under a subcategory seems fine to me, FWIW.

[chandlerc]

Another possibility would be to have an attribute to specifically mark a function that is defined against some external ABI rather than against a declaration from a header file.

Or a combination of a subcategory and an attribute.

[AaronBallman]

Thank you for raising this! I think this depends at least somewhat on what expectations are set for -Wmissing-prototypes given Clang's utter lack of documentation on it. Some folks (like maybe @chandlerc) think it means "tell me about missing prototypes in situations where I'd want a prototype" and others (like maybe @MaskRay) think it means "tell me about any function which doesn't have a prototype", and I think both viewpoints are pretty reasonable.

A confounding factor here is that Clang emits -Wmissing-prototypes in C++ code while GCC only emits it for C code: https://godbolt.org/z/n9Wsa3Wzx -- GCC documents that you should use -Wmissing-declarations for C++ code instead, which Clang also implements, but with different behavior from GCC: https://godbolt.org/z/6PqPnajn7

So the whole situation is a bit of a mess. I think the purpose to -Wmissing-prototypes is to tell the developer when a function definition is externally available but has no non-defining declaration with a prototype, with the assumption that a declaration for the interface would appear in a header file, making that interface easier to call, and the prototype makes the interface easier to call correctly. From that perspective, it makes some sense to me that we would diagnose extern "C" function definitions with no prior declaration -- the function is externally available and has no prior declaration. However, I'm not convinced -Wmissing-prototypes makes sense in C++ at all; this seems like it should be under -Wmissing-declarations instead. In C++, all functions have prototypes; the same is not true in C and that matters. e.g.,

void f();

void uh_oh(void) {
  f(1, 2, 3); // Compiles fine, error at runtime, see below.
}

void f() { // Gets a -Wmissing-prototypes warning
}

This code has a problem in C and is perfectly fine in C++ (because you'd get the diagnostic you'd expect in that case).

I did some historical digging to see if this was considered when the feature was worked on. The diagnostic was added in f1b876d5 and came with no discussion or tests, but was exposed for C++ functions. So this behavior has been this way in Clang since 2009 but it's not clear to me that it was an intentional design decision.