r218166 broke building the C code output from the MIDL compiler

[ehsan]

Bugzilla Link	21027
Resolution	FIXED
Resolved on	Nov 03, 2014 15:30
Version	trunk
OS	All
CC	@majnemer,@zmodem,@jrmuizel,@nico,@rnk,@rinon

Extended Description

Such code includes:

HRESULT stdcall DllRegisterServer() { ... } HRESULT stdcall DllUnregisterServer() { .... }

Now, building with clang-cl emits an error because these functions are not defined in a system header.

[rnk]

I downgraded this to a warning in r218394 and forgot to close this out, but I've gone further in r221184 by making it not warn at all for unprototyped definitions.

[rnk]

If not, the best we can do is probably to turn this into a regular warning for __stdcall. Or, since C99 warns on implicit calls without declaration, maybe we can not emit this diagnostic in C99 mode.

Not warning in C99 mode won't solve this problem. I think the right thing to do is to add a new warning like "Usage of calling convention %0 on function with no prototype is a Microsoft extension" and emit that instead of err_cconv_knr/warn_cconv_knr if MS extensions are enabled. WDYT?

IMO we should keep the wording mostly the same and downgrade it back to a warning. I'll send a patch.

[ehsan]

If not, the best we can do is probably to turn this into a regular warning for __stdcall. Or, since C99 warns on implicit calls without declaration, maybe we can not emit this diagnostic in C99 mode.

Not warning in C99 mode won't solve this problem. I think the right thing to do is to add a new warning like "Usage of calling convention %0 on function with no prototype is a Microsoft extension" and emit that instead of err_cconv_knr/warn_cconv_knr if MS extensions are enabled. WDYT?

[nico]

Some notes on what the warning is supposed to catch.

Modern C says that a function definition with an empty parameter is a function taking 0 parameters. (In declarations, no parameter list just means "no information about parameters" and (void) means 0 parameters): http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1256.pdf 6.7.5.3/14 So there's no need to insert a fixit with (void), that's no behavior difference.

So what is this warning supposed to catch? Before C99, functions didn't require a declaration (and even in C99, it's only a warning in clang).

So you could write

void f() { g(1, 2, 3); }

But if g is implemented in another translation unit and has callee cleanups, that's bad since f() can't check if the parameters are right and the cleanups in g() might be wrong. But if it looks like this

void g(int, int, int); void f() { g(1, 2, 3); }

then it's likely that the declaration of g is from a header, and that that header is also present when g is compiled -- I think that's what the warning is supposed to catch. (I'm not sure since I didn't write the warning, I just extended it from fastcall to all callee cleanup conventions.)

I think in the cases discussed in this bug, the situation is

void __stdcall g() {} void f() { g(); }

which is also fine. But clang can't know that g is used in only this translation unit and that it's not called from elsewhere, so it thinks "there better be a prototype for this, that others can include"! I suppose we could not emit the diagnostic for static functions, but that won't help for the examples on this bug. I can't think of any other clever heuristics.

But maybe I'm misunderstanding what the warning's supposed to warn on?

If not, the best we can do is probably to turn this into a regular warning for __stdcall. Or, since C99 warns on implicit calls without declaration, maybe we can not emit this diagnostic in C99 mode.

[991901f3-cc14-4404-b340-165691b62a58]

We should relax the diagnostic, there is precedent for this in r211238.

[nico]

Yeah, it's firing on using the DLLDATA_ROUTINES macro too. I agree we need to do something.

[ehsan]

You can forward declare these as 'HRESULT __stdcall DllRegisterServer(void);' to fix the problem. Is that possible, or is the .h file generated by MIDL too?

This whole code is generated by the MIDL compiler from a bunch of .idl files.

If you can't fix it, as a workaround you can use -Wno-missing-prototype-for-cc, but if you do that, we probably shouldn't have it as a default error warning. =/

Yeah, I think right now clang-cl basically can't build the MIDL compiler output any longer. :(

[rnk]

You can forward declare these as 'HRESULT __stdcall DllRegisterServer(void);' to fix the problem. Is that possible, or is the .h file generated by MIDL too?

If you can't fix it, as a workaround you can use -Wno-missing-prototype-for-cc, but if you do that, we probably shouldn't have it as a default error warning. =/

Nico, should we emit a FIXME to insert void?

[ehsan]

(Note that the errors occur in the generated dlldata.c files.)

[ehsan]

Nico, can you please suggest a way to fix this? We have some of this code in our tree which is how I noted this regression. Thanks!