r316245 - Implement current CWG direction for support of arrays of unknown bounds in

Richard Smith via cfe-commits cfe-commits at lists.llvm.org
Fri Oct 20 15:56:26 PDT 2017


Author: rsmith
Date: Fri Oct 20 15:56:25 2017
New Revision: 316245

URL: http://llvm.org/viewvc/llvm-project?rev=316245&view=rev
Log:
Implement current CWG direction for support of arrays of unknown bounds in
constant expressions.

We permit array-to-pointer decay on such arrays, but disallow pointer
arithmetic (since we do not know whether it will have defined behavior).

This is based on r311970 and r301822 (the former by me and the latter by Robert
Haberlach). Between then and now, two things have changed: we have committee
feedback indicating that this is indeed the right direction, and the code
broken by this change has been fixed.

This is necessary in C++17 to continue accepting certain forms of non-type
template argument involving arrays of unknown bound.

Added:
    cfe/trunk/test/SemaCXX/constexpr-array-unknown-bound.cpp
Modified:
    cfe/trunk/include/clang/Basic/DiagnosticASTKinds.td
    cfe/trunk/include/clang/Basic/DiagnosticIDs.h
    cfe/trunk/lib/AST/ExprConstant.cpp
    cfe/trunk/test/SemaCXX/constant-expression-cxx11.cpp
    cfe/trunk/test/SemaTemplate/temp_arg_nontype_cxx1z.cpp

Modified: cfe/trunk/include/clang/Basic/DiagnosticASTKinds.td
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticASTKinds.td?rev=316245&r1=316244&r2=316245&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/DiagnosticASTKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticASTKinds.td Fri Oct 20 15:56:25 2017
@@ -127,6 +127,10 @@ def note_constexpr_access_null : Note<
 def note_constexpr_access_past_end : Note<
   "%select{read of|assignment to|increment of|decrement of}0 "
   "dereferenced one-past-the-end pointer is not allowed in a constant expression">;
+def note_constexpr_access_unsized_array : Note<
+  "%select{read of|assignment to|increment of|decrement of}0 "
+  "pointer to element of array without known bound "
+  "is not allowed in a constant expression">;
 def note_constexpr_access_inactive_union_member : Note<
   "%select{read of|assignment to|increment of|decrement of}0 "
   "member %1 of union with %select{active member %3|no active member}2 "
@@ -154,6 +158,11 @@ def note_constexpr_baa_insufficient_alig
 def note_constexpr_baa_value_insufficient_alignment : Note<
   "value of the aligned pointer (%0) is not a multiple of the asserted %1 "
   "%plural{1:byte|:bytes}1">;
+def note_constexpr_unsupported_unsized_array : Note<
+  "array-to-pointer decay of array member without known bound is not supported">;
+def note_constexpr_unsized_array_indexed : Note<
+  "indexing of array without known bound is not allowed "
+  "in a constant expression">;
 
 def warn_integer_constant_overflow : Warning<
   "overflow in expression; result is %0 with type %1">,

Modified: cfe/trunk/include/clang/Basic/DiagnosticIDs.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticIDs.h?rev=316245&r1=316244&r2=316245&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/DiagnosticIDs.h (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticIDs.h Fri Oct 20 15:56:25 2017
@@ -34,7 +34,7 @@ namespace clang {
       DIAG_SIZE_SERIALIZATION =  120,
       DIAG_SIZE_LEX           =  400,
       DIAG_SIZE_PARSE         =  500,
-      DIAG_SIZE_AST           =  110,
+      DIAG_SIZE_AST           =  150,
       DIAG_SIZE_COMMENT       =  100,
       DIAG_SIZE_CROSSTU       =  100,
       DIAG_SIZE_SEMA          = 3500,

Modified: cfe/trunk/lib/AST/ExprConstant.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ExprConstant.cpp?rev=316245&r1=316244&r2=316245&view=diff
==============================================================================
--- cfe/trunk/lib/AST/ExprConstant.cpp (original)
+++ cfe/trunk/lib/AST/ExprConstant.cpp Fri Oct 20 15:56:25 2017
@@ -62,7 +62,13 @@ namespace {
   static QualType getType(APValue::LValueBase B) {
     if (!B) return QualType();
     if (const ValueDecl *D = B.dyn_cast<const ValueDecl*>())
-      return D->getType();
+      // FIXME: It's unclear where we're supposed to take the type from, and
+      // this actually matters for arrays of unknown bound. Using the type of
+      // the most recent declaration isn't clearly correct in general. Eg:
+      //
+      // extern int arr[]; void f() { extern int arr[3]; };
+      // constexpr int *p = &arr[1]; // valid?
+      return cast<ValueDecl>(D->getMostRecentDecl())->getType();
 
     const Expr *Base = B.get<const Expr*>();
 
@@ -141,6 +147,12 @@ namespace {
     return E && E->getType()->isPointerType() && tryUnwrapAllocSizeCall(E);
   }
 
+  /// The bound to claim that an array of unknown bound has.
+  /// The value in MostDerivedArraySize is undefined in this case. So, set it
+  /// to an arbitrary value that's likely to loudly break things if it's used.
+  static const uint64_t AssumedSizeForUnsizedArray =
+      std::numeric_limits<uint64_t>::max() / 2;
+
   /// Determines if an LValue with the given LValueBase will have an unsized
   /// array in its designator.
   /// Find the path length and type of the most-derived subobject in the given
@@ -148,7 +160,8 @@ namespace {
   static unsigned
   findMostDerivedSubobject(ASTContext &Ctx, APValue::LValueBase Base,
                            ArrayRef<APValue::LValuePathEntry> Path,
-                           uint64_t &ArraySize, QualType &Type, bool &IsArray) {
+                           uint64_t &ArraySize, QualType &Type, bool &IsArray,
+                           bool &FirstEntryIsUnsizedArray) {
     // This only accepts LValueBases from APValues, and APValues don't support
     // arrays that lack size info.
     assert(!isBaseAnAllocSizeCall(Base) &&
@@ -158,12 +171,18 @@ namespace {
 
     for (unsigned I = 0, N = Path.size(); I != N; ++I) {
       if (Type->isArrayType()) {
-        const ConstantArrayType *CAT =
-            cast<ConstantArrayType>(Ctx.getAsArrayType(Type));
-        Type = CAT->getElementType();
-        ArraySize = CAT->getSize().getZExtValue();
+        const ArrayType *AT = Ctx.getAsArrayType(Type);
+        Type = AT->getElementType();
         MostDerivedLength = I + 1;
         IsArray = true;
+
+        if (auto *CAT = dyn_cast<ConstantArrayType>(AT)) {
+          ArraySize = CAT->getSize().getZExtValue();
+        } else {
+          assert(I == 0 && "unexpected unsized array designator");
+          FirstEntryIsUnsizedArray = true;
+          ArraySize = AssumedSizeForUnsizedArray;
+        }
       } else if (Type->isAnyComplexType()) {
         const ComplexType *CT = Type->castAs<ComplexType>();
         Type = CT->getElementType();
@@ -246,10 +265,12 @@ namespace {
         Entries.insert(Entries.end(), VEntries.begin(), VEntries.end());
         if (V.getLValueBase()) {
           bool IsArray = false;
+          bool FirstIsUnsizedArray = false;
           MostDerivedPathLength = findMostDerivedSubobject(
               Ctx, V.getLValueBase(), V.getLValuePath(), MostDerivedArraySize,
-              MostDerivedType, IsArray);
+              MostDerivedType, IsArray, FirstIsUnsizedArray);
           MostDerivedIsArrayElement = IsArray;
+          FirstEntryIsAnUnsizedArray = FirstIsUnsizedArray;
         }
       }
     }
@@ -318,7 +339,7 @@ namespace {
       // The value in MostDerivedArraySize is undefined in this case. So, set it
       // to an arbitrary value that's likely to loudly break things if it's
       // used.
-      MostDerivedArraySize = std::numeric_limits<uint64_t>::max() / 2;
+      MostDerivedArraySize = AssumedSizeForUnsizedArray;
       MostDerivedPathLength = Entries.size();
     }
     /// Update this designator to refer to the given base or member of this
@@ -350,6 +371,7 @@ namespace {
       MostDerivedArraySize = 2;
       MostDerivedPathLength = Entries.size();
     }
+    void diagnoseUnsizedArrayPointerArithmetic(EvalInfo &Info, const Expr *E);
     void diagnosePointerArithmetic(EvalInfo &Info, const Expr *E,
                                    const APSInt &N);
     /// Add N to the address of this subobject.
@@ -357,6 +379,7 @@ namespace {
       if (Invalid || !N) return;
       uint64_t TruncatedN = N.extOrTrunc(64).getZExtValue();
       if (isMostDerivedAnUnsizedArray()) {
+        diagnoseUnsizedArrayPointerArithmetic(Info, E);
         // Can't verify -- trust that the user is doing the right thing (or if
         // not, trust that the caller will catch the bad behavior).
         // FIXME: Should we reject if this overflows, at least?
@@ -1094,9 +1117,19 @@ bool SubobjectDesignator::checkSubobject
     setInvalid();
     return false;
   }
+  // Note, we do not diagnose if isMostDerivedAnUnsizedArray(), because there
+  // must actually be at least one array element; even a VLA cannot have a
+  // bound of zero. And if our index is nonzero, we already had a CCEDiag.
   return true;
 }
 
+void SubobjectDesignator::diagnoseUnsizedArrayPointerArithmetic(EvalInfo &Info,
+                                                                const Expr *E) {
+  Info.CCEDiag(E, diag::note_constexpr_unsized_array_indexed);
+  // Do not set the designator as invalid: we can represent this situation,
+  // and correct handling of __builtin_object_size requires us to do so.
+}
+
 void SubobjectDesignator::diagnosePointerArithmetic(EvalInfo &Info,
                                                     const Expr *E,
                                                     const APSInt &N) {
@@ -1240,8 +1273,6 @@ namespace {
                     IsNullPtr);
       else {
         assert(!InvalidBase && "APValues can't handle invalid LValue bases");
-        assert(!Designator.FirstEntryIsAnUnsizedArray &&
-               "Unsized array with a valid base?");
         V = APValue(Base, Offset, Designator.Entries,
                     Designator.IsOnePastTheEnd, CallIndex, IsNullPtr);
       }
@@ -1314,10 +1345,14 @@ namespace {
       if (checkSubobject(Info, E, isa<FieldDecl>(D) ? CSK_Field : CSK_Base))
         Designator.addDeclUnchecked(D, Virtual);
     }
-    void addUnsizedArray(EvalInfo &Info, QualType ElemTy) {
-      assert(Designator.Entries.empty() && getType(Base)->isPointerType());
-      assert(isBaseAnAllocSizeCall(Base) &&
-             "Only alloc_size bases can have unsized arrays");
+    void addUnsizedArray(EvalInfo &Info, const Expr *E, QualType ElemTy) {
+      if (!Designator.Entries.empty()) {
+        Info.CCEDiag(E, diag::note_constexpr_unsupported_unsized_array);
+        Designator.setInvalid();
+        return;
+      }
+
+      assert(getType(Base)->isPointerType() || getType(Base)->isArrayType());
       Designator.FirstEntryIsAnUnsizedArray = true;
       Designator.addUnsizedArrayUnchecked(ElemTy);
     }
@@ -2624,10 +2659,12 @@ findSubobject(EvalInfo &Info, const Expr
   if (Sub.Invalid)
     // A diagnostic will have already been produced.
     return handler.failed();
-  if (Sub.isOnePastTheEnd()) {
+  if (Sub.isOnePastTheEnd() || Sub.isMostDerivedAnUnsizedArray()) {
     if (Info.getLangOpts().CPlusPlus11)
-      Info.FFDiag(E, diag::note_constexpr_access_past_end)
-        << handler.AccessKind;
+      Info.FFDiag(E, Sub.isOnePastTheEnd()
+                         ? diag::note_constexpr_access_past_end
+                         : diag::note_constexpr_access_unsized_array)
+          << handler.AccessKind;
     else
       Info.FFDiag(E);
     return handler.failed();
@@ -5487,7 +5524,7 @@ static bool evaluateLValueAsAllocSize(Ev
   Result.setInvalid(E);
 
   QualType Pointee = E->getType()->castAs<PointerType>()->getPointeeType();
-  Result.addUnsizedArray(Info, Pointee);
+  Result.addUnsizedArray(Info, E, Pointee);
   return true;
 }
 
@@ -5697,7 +5734,8 @@ bool PointerExprEvaluator::VisitCastExpr
       return true;
     }
   }
-  case CK_ArrayToPointerDecay:
+
+  case CK_ArrayToPointerDecay: {
     if (SubExpr->isGLValue()) {
       if (!evaluateLValue(SubExpr, Result))
         return false;
@@ -5708,12 +5746,13 @@ bool PointerExprEvaluator::VisitCastExpr
         return false;
     }
     // The result is a pointer to the first element of the array.
-    if (const ConstantArrayType *CAT
-          = Info.Ctx.getAsConstantArrayType(SubExpr->getType()))
+    auto *AT = Info.Ctx.getAsArrayType(SubExpr->getType());
+    if (auto *CAT = dyn_cast<ConstantArrayType>(AT))
       Result.addArray(Info, E, CAT);
     else
-      Result.Designator.setInvalid();
+      Result.addUnsizedArray(Info, E, AT->getElementType());
     return true;
+  }
 
   case CK_FunctionToPointerDecay:
     return evaluateLValue(SubExpr, Result);
@@ -5780,7 +5819,7 @@ bool PointerExprEvaluator::visitNonBuilt
 
   Result.setInvalid(E);
   QualType PointeeTy = E->getType()->castAs<PointerType>()->getPointeeType();
-  Result.addUnsizedArray(Info, PointeeTy);
+  Result.addUnsizedArray(Info, E, PointeeTy);
   return true;
 }
 
@@ -7341,7 +7380,8 @@ static const Expr *ignorePointerCastsAnd
 /// Please note: this function is specialized for how __builtin_object_size
 /// views "objects".
 ///
-/// If this encounters an invalid RecordDecl, it will always return true.
+/// If this encounters an invalid RecordDecl or otherwise cannot determine the
+/// correct result, it will always return true.
 static bool isDesignatorAtObjectEnd(const ASTContext &Ctx, const LValue &LVal) {
   assert(!LVal.Designator.Invalid);
 
@@ -7372,9 +7412,8 @@ static bool isDesignatorAtObjectEnd(cons
   unsigned I = 0;
   QualType BaseType = getType(Base);
   if (LVal.Designator.FirstEntryIsAnUnsizedArray) {
-    assert(isBaseAnAllocSizeCall(Base) &&
-           "Unsized array in non-alloc_size call?");
-    // If this is an alloc_size base, we should ignore the initial array index
+    // If we don't know the array bound, conservatively assume we're looking at
+    // the final array element.
     ++I;
     BaseType = BaseType->castAs<PointerType>()->getPointeeType();
   }

Modified: cfe/trunk/test/SemaCXX/constant-expression-cxx11.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/constant-expression-cxx11.cpp?rev=316245&r1=316244&r2=316245&view=diff
==============================================================================
--- cfe/trunk/test/SemaCXX/constant-expression-cxx11.cpp (original)
+++ cfe/trunk/test/SemaCXX/constant-expression-cxx11.cpp Fri Oct 20 15:56:25 2017
@@ -604,20 +604,31 @@ static_assert(NATDCArray{}[1][1].n == 0,
 
 }
 
-// Tests for indexes into arrays of unknown bounds.
+// Per current CWG direction, we reject any cases where pointer arithmetic is
+// not statically known to be valid.
 namespace ArrayOfUnknownBound {
-  // This is a corner case of the language where it's not clear whether this
-  // should be an error: When we see the initializer for Z::a, the bounds of
-  // Z::b aren't known yet, but they will be known by the end of the translation
-  // unit, so the compiler can in theory check the indexing into Z::b.
-  // For the time being, as long as this is unclear, we want to make sure that
-  // this compiles.
-  struct Z {
-    static const void *const a[];
-    static const void *const b[];
+  extern int arr[];
+  constexpr int *a = arr;
+  constexpr int *b = &arr[0];
+  static_assert(a == b, "");
+  constexpr int *c = &arr[1]; // expected-error {{constant}} expected-note {{indexing of array without known bound}}
+  constexpr int *d = &a[1]; // expected-error {{constant}} expected-note {{indexing of array without known bound}}
+  constexpr int *e = a + 1; // expected-error {{constant}} expected-note {{indexing of array without known bound}}
+
+  struct X {
+    int a;
+    int b[]; // expected-warning {{C99}}
   };
-  constexpr const void *Z::a[] = {&b[0], &b[1]};
-  constexpr const void *Z::b[] = {&a[0], &a[1]};
+  extern X x;
+  constexpr int *xb = x.b; // expected-error {{constant}} expected-note {{not supported}}
+
+  struct Y { int a; };
+  extern Y yarr[];
+  constexpr Y *p = yarr;
+  constexpr int *q = &p->a;
+
+  extern const int carr[]; // expected-note {{here}}
+  constexpr int n = carr[0]; // expected-error {{constant}} expected-note {{non-constexpr variable}}
 }
 
 namespace DependentValues {

Added: cfe/trunk/test/SemaCXX/constexpr-array-unknown-bound.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/constexpr-array-unknown-bound.cpp?rev=316245&view=auto
==============================================================================
--- cfe/trunk/test/SemaCXX/constexpr-array-unknown-bound.cpp (added)
+++ cfe/trunk/test/SemaCXX/constexpr-array-unknown-bound.cpp Fri Oct 20 15:56:25 2017
@@ -0,0 +1,26 @@
+// RUN: %clang_cc1 %s -Wno-uninitialized -std=c++1z -fsyntax-only -verify
+
+const extern int arr[];
+constexpr auto p = arr; // ok
+constexpr int f(int i) {return p[i];} // expected-note {{read of dereferenced one-past-the-end pointer}}
+
+constexpr int arr[] {1, 2, 3};
+constexpr auto p2 = arr + 2; // ok
+constexpr int x = f(2); // ok
+constexpr int y = f(3); // expected-error {{constant expression}}
+// expected-note-re at -1 {{in call to 'f({{.*}})'}}
+
+// FIXME: consider permitting this case
+struct A {int m[];} a;
+constexpr auto p3 = a.m; // expected-error {{constant expression}} expected-note {{without known bound}}
+constexpr auto p4 = a.m + 1; // expected-error {{constant expression}} expected-note {{without known bound}}
+
+void g(int i) {
+  int arr[i];
+  constexpr auto *p = arr + 2; // expected-error {{constant expression}} expected-note {{without known bound}}
+
+  // FIXME: Give a better diagnostic here. The issue is that computing
+  // sizeof(*arr2) within the array indexing fails due to the VLA.
+  int arr2[2][i];
+  constexpr int m = ((void)arr2[2], 0); // expected-error {{constant expression}}
+}

Modified: cfe/trunk/test/SemaTemplate/temp_arg_nontype_cxx1z.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaTemplate/temp_arg_nontype_cxx1z.cpp?rev=316245&r1=316244&r2=316245&view=diff
==============================================================================
--- cfe/trunk/test/SemaTemplate/temp_arg_nontype_cxx1z.cpp (original)
+++ cfe/trunk/test/SemaTemplate/temp_arg_nontype_cxx1z.cpp Fri Oct 20 15:56:25 2017
@@ -23,6 +23,9 @@ namespace Array {
   A<const char*, &(&x)[1]> h; // expected-error {{refers to subobject '&x + 1'}}
   A<const char*, 0> i; // expected-error {{not allowed in a converted constant}}
   A<const char*, nullptr> j;
+
+  extern char aub[];
+  A<char[], aub> k;
 }
 
 namespace Function {




More information about the cfe-commits mailing list