This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG6d09aaecdfe5: Revert "[clang] Add early exit when 
checking for const init of arrays." (authored by adamcz).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113599

Files:
  clang/lib/AST/ExprConstant.cpp
  clang/test/SemaCXX/PR51712-large-array-constexpr-check-oom.cpp


Index: clang/test/SemaCXX/PR51712-large-array-constexpr-check-oom.cpp
===================================================================
--- clang/test/SemaCXX/PR51712-large-array-constexpr-check-oom.cpp
+++ /dev/null
@@ -1,11 +0,0 @@
-// REQUIRES: shell
-// UNSUPPORTED: win32
-// RUN: ulimit -v 1048576
-// RUN: %clang_cc1 -std=c++11 -fsyntax-only -verify -triple=x86_64 %s
-// expected-no-diagnostics
-
-// This used to require too much memory and crash with OOM.
-struct {
-  int a, b, c, d;
-} arr[1<<30];
-
Index: clang/lib/AST/ExprConstant.cpp
===================================================================
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -10596,55 +10596,28 @@
   bool HadZeroInit = Value->hasValue();
 
   if (const ConstantArrayType *CAT = Info.Ctx.getAsConstantArrayType(Type)) {
-    unsigned FinalSize = CAT->getSize().getZExtValue();
+    unsigned N = CAT->getSize().getZExtValue();
 
     // Preserve the array filler if we had prior zero-initialization.
     APValue Filler =
       HadZeroInit && Value->hasArrayFiller() ? Value->getArrayFiller()
                                              : APValue();
 
-    *Value = APValue(APValue::UninitArray(), 0, FinalSize);
-    if (FinalSize == 0)
-      return true;
+    *Value = APValue(APValue::UninitArray(), N, N);
+
+    if (HadZeroInit)
+      for (unsigned I = 0; I != N; ++I)
+        Value->getArrayInitializedElt(I) = Filler;
 
+    // Initialize the elements.
     LValue ArrayElt = Subobject;
     ArrayElt.addArray(Info, E, CAT);
-    // We do the whole initialization in two passes, first for just one 
element,
-    // then for the whole array. It's possible we may find out we can't do 
const
-    // init in the first pass, in which case we avoid allocating a potentially
-    // large array. We don't do more passes because expanding array requires
-    // copying the data, which is wasteful.
-    for (const unsigned N : {1u, FinalSize}) {
-      unsigned OldElts = Value->getArrayInitializedElts();
-      if (OldElts == N)
-        break;
-
-      // Expand the array to appropriate size.
-      APValue NewValue(APValue::UninitArray(), N, FinalSize);
-      for (unsigned I = 0; I < OldElts; ++I)
-        NewValue.getArrayInitializedElt(I).swap(
-            Value->getArrayInitializedElt(I));
-      Value->swap(NewValue);
-
-      if (HadZeroInit)
-        for (unsigned I = OldElts; I < N; ++I)
-          Value->getArrayInitializedElt(I) = Filler;
-
-      // Initialize the elements.
-      for (unsigned I = OldElts; I < N; ++I) {
-        if (!VisitCXXConstructExpr(E, ArrayElt,
-                                   &Value->getArrayInitializedElt(I),
-                                   CAT->getElementType()) ||
-            !HandleLValueArrayAdjustment(Info, E, ArrayElt,
-                                         CAT->getElementType(), 1))
-          return false;
-        // When checking for const initilization any diagnostic is considered
-        // an error.
-        if (Info.EvalStatus.Diag && !Info.EvalStatus.Diag->empty() &&
-            !Info.keepEvaluatingAfterFailure())
-          return false;
-      }
-    }
+    for (unsigned I = 0; I != N; ++I)
+      if (!VisitCXXConstructExpr(E, ArrayElt, 
&Value->getArrayInitializedElt(I),
+                                 CAT->getElementType()) ||
+          !HandleLValueArrayAdjustment(Info, E, ArrayElt, 
CAT->getElementType(),
+                                       1))
+        return false;
 
     return true;
   }


Index: clang/test/SemaCXX/PR51712-large-array-constexpr-check-oom.cpp
===================================================================
--- clang/test/SemaCXX/PR51712-large-array-constexpr-check-oom.cpp
+++ /dev/null
@@ -1,11 +0,0 @@
-// REQUIRES: shell
-// UNSUPPORTED: win32
-// RUN: ulimit -v 1048576
-// RUN: %clang_cc1 -std=c++11 -fsyntax-only -verify -triple=x86_64 %s
-// expected-no-diagnostics
-
-// This used to require too much memory and crash with OOM.
-struct {
-  int a, b, c, d;
-} arr[1<<30];
-
Index: clang/lib/AST/ExprConstant.cpp
===================================================================
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -10596,55 +10596,28 @@
   bool HadZeroInit = Value->hasValue();
 
   if (const ConstantArrayType *CAT = Info.Ctx.getAsConstantArrayType(Type)) {
-    unsigned FinalSize = CAT->getSize().getZExtValue();
+    unsigned N = CAT->getSize().getZExtValue();
 
     // Preserve the array filler if we had prior zero-initialization.
     APValue Filler =
       HadZeroInit && Value->hasArrayFiller() ? Value->getArrayFiller()
                                              : APValue();
 
-    *Value = APValue(APValue::UninitArray(), 0, FinalSize);
-    if (FinalSize == 0)
-      return true;
+    *Value = APValue(APValue::UninitArray(), N, N);
+
+    if (HadZeroInit)
+      for (unsigned I = 0; I != N; ++I)
+        Value->getArrayInitializedElt(I) = Filler;
 
+    // Initialize the elements.
     LValue ArrayElt = Subobject;
     ArrayElt.addArray(Info, E, CAT);
-    // We do the whole initialization in two passes, first for just one element,
-    // then for the whole array. It's possible we may find out we can't do const
-    // init in the first pass, in which case we avoid allocating a potentially
-    // large array. We don't do more passes because expanding array requires
-    // copying the data, which is wasteful.
-    for (const unsigned N : {1u, FinalSize}) {
-      unsigned OldElts = Value->getArrayInitializedElts();
-      if (OldElts == N)
-        break;
-
-      // Expand the array to appropriate size.
-      APValue NewValue(APValue::UninitArray(), N, FinalSize);
-      for (unsigned I = 0; I < OldElts; ++I)
-        NewValue.getArrayInitializedElt(I).swap(
-            Value->getArrayInitializedElt(I));
-      Value->swap(NewValue);
-
-      if (HadZeroInit)
-        for (unsigned I = OldElts; I < N; ++I)
-          Value->getArrayInitializedElt(I) = Filler;
-
-      // Initialize the elements.
-      for (unsigned I = OldElts; I < N; ++I) {
-        if (!VisitCXXConstructExpr(E, ArrayElt,
-                                   &Value->getArrayInitializedElt(I),
-                                   CAT->getElementType()) ||
-            !HandleLValueArrayAdjustment(Info, E, ArrayElt,
-                                         CAT->getElementType(), 1))
-          return false;
-        // When checking for const initilization any diagnostic is considered
-        // an error.
-        if (Info.EvalStatus.Diag && !Info.EvalStatus.Diag->empty() &&
-            !Info.keepEvaluatingAfterFailure())
-          return false;
-      }
-    }
+    for (unsigned I = 0; I != N; ++I)
+      if (!VisitCXXConstructExpr(E, ArrayElt, &Value->getArrayInitializedElt(I),
+                                 CAT->getElementType()) ||
+          !HandleLValueArrayAdjustment(Info, E, ArrayElt, CAT->getElementType(),
+                                       1))
+        return false;
 
     return true;
   }
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
  • [PATCH] D113599: Revert ... Adam Czachorowski via Phabricator via cfe-commits
    • [PATCH] D113599: Re... Adam Czachorowski via Phabricator via cfe-commits

Reply via email to