[PATCH] D64883: Add new warning -Walloca for use of builtin alloca function

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.


================
Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:2776
+def warn_alloca : Warning<
+  "use of builtin function %0">,
+  InGroup<DiagGroup<"alloca">>, DefaultIgnore;
----------------
george.burgess.iv wrote:
> nit: I'd just say "use of function '%0'" here; "builtin" doesn't really add much.
> 
> I also wonder if we should be saying anything more than "we found a use of this function." Looks like GCC doesn't (https://godbolt.org/z/sYs_8G), but since this warning is sort of opinionated in itself, might it be better to expand this to "use of '%0' is discouraged"?
> 
> WDYT, Aaron?
What is the purpose to this diagnostic, aside from GCC compatibility? What does it protect against?

If there's a reason users should not use alloc(), it would be better for the diagnostic to spell it out.

Btw, I'm okay with this being default-off because the GCC warning is as well. I'm mostly hoping we can do better with our diagnostic wording.


================
Comment at: clang/lib/Sema/SemaChecking.cpp:1172
       return ExprError();
+    LLVM_FALLTHROUGH;
+  case Builtin::BI__builtin_alloca:
----------------
Do we want to warn on all uses of alloca(), or just the ones that get past the call to `SemaBuiltinAllocaWithAlign()`?


================
Comment at: clang/test/Sema/warn-alloca.c:1
+// RUN: %clang_cc1 -fsyntax-only -verify -Walloca %s
+
----------------
I'd appreciate a test demonstrating no warnings if `-Wall` is passed without an explicit `-Walloca`.


Repository:
  rC Clang

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

https://reviews.llvm.org/D64883