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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
