Origami404 created this revision.
Herald added a project: All.
Origami404 edited the summary of this revision.
Origami404 edited the summary of this revision.
Origami404 added reviewers: aaron.ballman, efriedma.
Origami404 added a subscriber: inclyc.
Origami404 published this revision for review.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

New behaviors:

`-Wvla`: produces portability warning for all VLAs like before
`-Wvla-portability`: not exists (equal to `-Wvla`)
`-Wvla-stack-allocation -Wno-vla`: only warns for VLAs that need stack 
allocation
`-Wvla -Wvla-stack-allocation`: warn for VLAs that need stack allocation, and 
give portablility warnings for other VLAs

Note that only one warning can be produced by one VLA because of
implementation complexity. Currently, VLAs are detected in
`BuildArrayType`, which will be used both by Parser and Tree Transformer.
And it passes the VLA-related warnings as an argument to other
functions. Producing multiple warnings for one VLA needs to change many
functions.

Fixes: https://github.com/llvm/llvm-project/issues/57098
Link: https://reviews.llvm.org/D132952

Co-authored-by: YingChi Long <m...@inclyc.cn>


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D137343

Files:
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaType.cpp
  clang/test/Sema/warn-vla.c

Index: clang/test/Sema/warn-vla.c
===================================================================
--- clang/test/Sema/warn-vla.c
+++ clang/test/Sema/warn-vla.c
@@ -1,12 +1,39 @@
-// RUN: %clang_cc1 -std=c99 -fsyntax-only -verify -Wvla %s
-// RUN: %clang_cc1 -std=c89 -fsyntax-only -verify -Wvla %s
+// RUN: %clang_cc1 -std=c99 -fsyntax-only -verify=expected,no-fallback-c99 -Wvla -Wvla-stack-allocation %s
+// RUN: %clang_cc1 -std=c89 -fsyntax-only -verify=expected,no-fallback-c89 -Wvla -Wvla-stack-allocation %s
+// RUN: %clang_cc1 -std=c99 -fsyntax-only -verify=expected,fallback -Wvla %s
+// RUN: %clang_cc1 -std=c99 -fsyntax-only -verify=expected,only-stack-allocation -Wvla-stack-allocation -Wno-vla %s
 
 void test1(int n) {
-  int v[n]; // expected-warning {{variable length array}}
+  int v[n]; /* no-fallback-c89-warning {{variable length arrays are a C99 feature}}
+               no-fallback-c99-warning {{variable length array that may require stack allocation used}}
+               fallback-warning {{variable length array used}}
+               only-stack-allocation-warning {{variable length array that may require stack allocation used}}
+            */
 }
 
-void test2(int n, int v[n]) { // expected-warning {{variable length array}}
+void test2(int n, int v[n]) { /* no-fallback-c89-warning {{variable length arrays are a C99 feature}}
+                                 no-fallback-c99-warning {{variable length array used}}
+                                 fallback-warning {{variable length array used}}
+                              */
 }
 
-void test3(int n, int v[n]); // expected-warning {{variable length array}}
+void test3(int n, int v[n]); /* no-fallback-c89-warning {{variable length arrays are a C99 feature}}
+                                no-fallback-c99-warning {{variable length array used}}
+                                fallback-warning {{variable length array used}}
+                              */
 
+int test4_num;
+typedef int Test4[test4_num]; /* no-fallback-c89-warning {{variable length arrays are a C99 feature}} 
+                                 no-fallback-c99-warning {{variable length array used}}
+                                 fallback-warning {{variable length array used}}
+                                 expected-error {{variable length array declaration not allowed at file scope}}
+                              */
+
+void test5() {
+  typedef int type[test4_num]; /* no-fallback-c89-warning {{variable length arrays are a C99 feature}} 
+                                  no-fallback-c99-warning {{variable length array that may require stack allocation used}}
+                                  fallback-warning {{variable length array used}}
+                                  only-stack-allocation-warning {{variable length array that may require stack allocation used}}
+                                */
+
+}
Index: clang/lib/Sema/SemaType.cpp
===================================================================
--- clang/lib/Sema/SemaType.cpp
+++ clang/lib/Sema/SemaType.cpp
@@ -22,6 +22,7 @@
 #include "clang/AST/Type.h"
 #include "clang/AST/TypeLoc.h"
 #include "clang/AST/TypeLocVisitor.h"
+#include "clang/Basic/DiagnosticSema.h"
 #include "clang/Basic/PartialDiagnostic.h"
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/Specifiers.h"
@@ -2530,7 +2531,8 @@
     return QualType();
   }
 
-  // VLAs always produce at least a -Wvla diagnostic, sometimes an error.
+  // VLAs always produce at least a -Wvla-portability or -Wvla-stack-allocation
+  // diagnostic, sometimes an error.
   unsigned VLADiag;
   bool VLAIsError;
   if (getLangOpts().OpenCL) {
@@ -2538,7 +2540,26 @@
     VLADiag = diag::err_opencl_vla;
     VLAIsError = true;
   } else if (getLangOpts().C99) {
-    VLADiag = diag::warn_vla_used;
+    // after C99, VLA is no longer an extension.
+    if (getCurScope()->getFlags() == Scope::ScopeFlags::DeclScope) {
+      // VLA in file scope typedef will have an error, and should not have a
+      // warning of portability. But for backward compatibility, we preform the
+      // exact same action like before (which will give an warning of "vla
+      // used").
+      VLADiag = diag::warn_vla_portability;
+    } else if (getCurScope()->isFunctionPrototypeScope()) {
+      // VLA in function prototype is acceptable by C2x standard
+      // so just give a protability warning
+      VLADiag = diag::warn_vla_portability;
+    } else {
+      // in other case, a VLA will cause stack allocation
+      // if -Wvla-stack-allocation is ignored, fallback to
+      // -Wvla-protability
+      if (getDiagnostics().isIgnored(diag::warn_vla_stack_allocation, Loc))
+        VLADiag = diag::warn_vla_portability;
+      else
+        VLADiag = diag::warn_vla_stack_allocation;
+    }
     VLAIsError = false;
   } else if (isSFINAEContext()) {
     VLADiag = diag::err_vla_in_sfinae;
@@ -2547,6 +2568,7 @@
     VLADiag = diag::err_openmp_vla_in_task_untied;
     VLAIsError = true;
   } else {
+    // Before C99 and in most of platform, VLA is an extension.
     VLADiag = diag::ext_vla;
     VLAIsError = false;
   }
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===================================================================
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -131,8 +131,10 @@
 // C99 variable-length arrays
 def ext_vla : Extension<"variable length arrays are a C99 feature">,
   InGroup<VLAExtension>;
-def warn_vla_used : Warning<"variable length array used">,
+def warn_vla_portability : Warning<"variable length array used">,
   InGroup<VLA>, DefaultIgnore;
+def warn_vla_stack_allocation : Warning<"variable length array that may require stack allocation used">,
+  InGroup<VLAStackAllocation>, DefaultIgnore;
 def err_vla_in_sfinae : Error<
   "variable length array cannot be formed during template argument deduction">;
 def err_array_star_in_function_definition : Error<
Index: clang/include/clang/Basic/DiagnosticGroups.td
===================================================================
--- clang/include/clang/Basic/DiagnosticGroups.td
+++ clang/include/clang/Basic/DiagnosticGroups.td
@@ -828,6 +828,7 @@
 def VexingParse : DiagGroup<"vexing-parse">;
 def VLAExtension : DiagGroup<"vla-extension">;
 def VLA : DiagGroup<"vla", [VLAExtension]>;
+def VLAStackAllocation : DiagGroup<"vla-stack-allocation", [VLA]>;
 def VolatileRegisterVar : DiagGroup<"volatile-register-var">;
 def Visibility : DiagGroup<"visibility">;
 def ZeroLengthArray : DiagGroup<"zero-length-array">;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to