aaron.ballman added a comment. In D156565#4654818 <https://reviews.llvm.org/D156565#4654818>, @aaron.ballman wrote:
> In D156565#4654773 <https://reviews.llvm.org/D156565#4654773>, @nathanchance > wrote: > >> Is it expected that this introduces a warning for C code, as the commit >> message and tests appear to only affect C++? A trivial example from the >> Linux kernel: >> >> https://elixir.bootlin.com/linux/v6.5.8/source/tools/lib/bpf/btf_dump.c#L1678 >> >> #include <stddef.h> >> #include <stdio.h> >> #include <string.h> >> >> void foo(char *orig_name, char **cached_name, size_t dup_cnt) >> { >> const size_t max_len = 256; >> char new_name[max_len]; >> >> snprintf(new_name, max_len, "%s___%zu", orig_name, dup_cnt); >> *cached_name = strdup(new_name); >> } >> >> >> >> $ clang -std=gnu89 -Wall -fsyntax-only test.c >> test.c:8:16: warning: variable length arrays are a C99 feature >> [-Wvla-extension] >> 8 | char new_name[max_len]; >> | ^~~~~~~ >> 1 warning generated. > > No, that's unintended, I'll get that fixed. Thanks for letting me know! Unfortunately, this will require complicating the diagnostic groups slightly more. We don't have facilities that allow us to say that a single diagnostic is grouped under `-Wall` in one language mode but not another, and we don't have a way for diagnostic groups to share the same string (so we can't have `def VLACxxExtension : DiagGroup<"vla-extension", [VLAUseStaticAssert]>;` and `def VLAExtension : DiagGroup<"vla-extension", [VLACxxExtension]>;`). I think I will address this by adding `-Wvla-cxx-extension` and putting the C++ warnings under it, and that warning group will be added to `-Wall`. e.g., diff --git a/clang/include/clang/Basic/DiagnosticGroups.td b/clang/include/clang/Basic/DiagnosticGroups.td index dcdae38013d2..4cb792132d6e 100644 --- a/clang/include/clang/Basic/DiagnosticGroups.td +++ b/clang/include/clang/Basic/DiagnosticGroups.td @@ -849,7 +849,8 @@ def VariadicMacros : DiagGroup<"variadic-macros">; def VectorConversion : DiagGroup<"vector-conversion">; // clang specific def VexingParse : DiagGroup<"vexing-parse">; def VLAUseStaticAssert : DiagGroup<"vla-extension-static-assert">; -def VLAExtension : DiagGroup<"vla-extension", [VLAUseStaticAssert]>; +def VLACxxExtension : DiagGroup<"vla-cxx-extension", [VLAUseStaticAssert]>; +def VLAExtension : DiagGroup<"vla-extension", [VLACxxExtension]>; def VLA : DiagGroup<"vla", [VLAExtension]>; def VolatileRegisterVar : DiagGroup<"volatile-register-var">; def Visibility : DiagGroup<"visibility">; @@ -1086,7 +1087,8 @@ def Consumed : DiagGroup<"consumed">; // warning should be active _only_ when -Wall is passed in, mark it as // DefaultIgnore in addition to putting it here. def All : DiagGroup<"all", [Most, Parentheses, Switch, SwitchBool, - MisleadingIndentation, PackedNonPod, VLAExtension]>; + MisleadingIndentation, PackedNonPod, + VLACxxExtension]>; // Warnings that should be in clang-cl /w4. def : DiagGroup<"CL4", [All, Extra]>; diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index a4c1cb08de94..3bcbb003d6de 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -141,9 +141,9 @@ def ext_vla : Extension<"variable length arrays are a C99 feature">, // language modes, we warn as an extension but add the warning group to -Wall. def ext_vla_cxx : ExtWarn< "variable length arrays in C++ are a Clang extension">, - InGroup<VLAExtension>; + InGroup<VLACxxExtension>; def ext_vla_cxx_in_gnu_mode : Extension<ext_vla_cxx.Summary>, - InGroup<VLAExtension>; + InGroup<VLACxxExtension>; def ext_vla_cxx_static_assert : ExtWarn< "variable length arrays in C++ are a Clang extension; did you mean to use " "'static_assert'?">, InGroup<VLAUseStaticAssert>; Any concerns with this approach? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D156565/new/ https://reviews.llvm.org/D156565 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
