[PATCH] D122983: [C11/C2x] Change the behavior of the implicit function declaration warning

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith accepted this revision.
rsmith added a comment.
This revision is now accepted and ready to land.

LGTM; two non-blocking comments and a question.



================
Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:420-422
+def ext_implicit_function_decl_c99 : ExtWarn<
+  "implicit declaration of function %0 is invalid in C99 and later and "
+  "unsupported in C2x">, InGroup<ImplicitFunctionDeclare>, DefaultError;
----------------
The context for someone reading this diagnostic is that they're building in C99 / C11 / C17 mode, used an undeclared function, and (by default) saw this error. In that context, I think it may not be especially relevant to them that Clang would not even allow the error to be disabled if they built in C2x mode, so I'd be inclined to remove the "and unsupported in C2x" part from this diagnostic. Also, they probably didn't *intend* to implicitly declare a function, so perhaps we could replace this diagnostic with something less implementation-oriented and more user-oriented:

"call to undeclared function %0; ISO C99 and later do not support implicit function declarations"

(The phrasing "ISO C99 and later do not support" is what we usually use when we want to imply "but perhaps Clang does support" -- keeping in mind that this diagnostic might appear as either an error or a warning, and we want phrasing that works both ways.)

That said: this is orthogonal to the changes you're making here, so don't consider this comment to be blocking.


================
Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:698-700
 def ext_implicit_lib_function_decl : ExtWarn<
   "implicitly declaring library function '%0' with type %1">,
   InGroup<ImplicitFunctionDeclare>;
----------------
Hm, why is this one an `ExtWarn`? Are we really allowed to reject implicit declarations of library functions (in `-std=c89 -pedantic-errors` mode)?


================
Comment at: clang/lib/Sema/SemaDecl.cpp:15319-15321
   // OpenCL v2.0 s6.9.u - Implicit function declaration is not supported.
   else if (getLangOpts().OpenCL)
     diag_id = diag::err_opencl_implicit_function_decl;
----------------
Should we even be calling this function in OpenCL mode? It seems a bit inconsistent that we avoid calling this in C++ and C2x mode, and that we call it but error in OpenCL mode.

Maybe there should be a function on `LangOptions` to ask if implicit function declarations are permitted in the current language mode, to make it easy to opt out the right cases? (Happy for this to be a follow-on change if you agree.)


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

https://reviews.llvm.org/D122983