Arcoth updated this revision to Diff 97114. Arcoth added a comment. Simplified array-to-pointer conversion (simply add the array as unsized; let findCompleteObject and the SubobjectDesignator ctor do the work).
https://reviews.llvm.org/D32372 Files: include/clang/Basic/DiagnosticASTKinds.td lib/AST/ExprConstant.cpp test/SemaCXX/constexpr-array-unknown-bound.cpp
Index: test/SemaCXX/constexpr-array-unknown-bound.cpp =================================================================== --- /dev/null +++ test/SemaCXX/constexpr-array-unknown-bound.cpp @@ -0,0 +1,20 @@ +// 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@-1 {{in call to 'f({{.*}})'}} + +struct A {int m[];} a; +constexpr auto p3 = a.m; // ok +constexpr auto p4 = a.m + 1; // expected-error {{constant expression}} expected-note {{constant bound}} + +void g(int i) { + int arr[i]; + constexpr auto p = arr + 2; // expected-error {{constant expression}} expected-note {{constant bound}} +} Index: lib/AST/ExprConstant.cpp =================================================================== --- lib/AST/ExprConstant.cpp +++ lib/AST/ExprConstant.cpp @@ -148,22 +148,26 @@ 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 &isUnsizedArray) { // This only accepts LValueBases from APValues, and APValues don't support // arrays that lack size info. assert(!isBaseAnAllocSizeCall(Base) && "Unsized arrays shouldn't appear here"); unsigned MostDerivedLength = 0; Type = getType(Base); 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(); + if (auto AT = Ctx.getAsArrayType(Type)) { MostDerivedLength = I + 1; IsArray = true; + if (auto CAT = Ctx.getAsConstantArrayType(Type)) + ArraySize = CAT->getSize().getZExtValue(); + else { + ArraySize = 0; + isUnsizedArray = true; + } + Type = AT->getElementType(); } else if (Type->isAnyComplexType()) { const ComplexType *CT = Type->castAs<ComplexType>(); Type = CT->getElementType(); @@ -200,8 +204,9 @@ /// Is this a pointer one past the end of an object? unsigned IsOnePastTheEnd : 1; - /// Indicator of whether the first entry is an unsized array. - unsigned FirstEntryIsAnUnsizedArray : 1; + /// Indicator of whether the most-derived object is an unsized array (e.g. + /// of unknown bound). + unsigned MostDerivedIsAnUnsizedArray : 1; /// Indicator of whether the most-derived object is an array element. unsigned MostDerivedIsArrayElement : 1; @@ -231,25 +236,28 @@ explicit SubobjectDesignator(QualType T) : Invalid(false), IsOnePastTheEnd(false), - FirstEntryIsAnUnsizedArray(false), MostDerivedIsArrayElement(false), + MostDerivedIsAnUnsizedArray(false), MostDerivedIsArrayElement(false), MostDerivedPathLength(0), MostDerivedArraySize(0), MostDerivedType(T) {} SubobjectDesignator(ASTContext &Ctx, const APValue &V) : Invalid(!V.isLValue() || !V.hasLValuePath()), IsOnePastTheEnd(false), - FirstEntryIsAnUnsizedArray(false), MostDerivedIsArrayElement(false), + MostDerivedIsAnUnsizedArray(false), MostDerivedIsArrayElement(false), MostDerivedPathLength(0), MostDerivedArraySize(0) { assert(V.isLValue() && "Non-LValue used to make an LValue designator?"); if (!Invalid) { IsOnePastTheEnd = V.isLValueOnePastTheEnd(); ArrayRef<PathEntry> VEntries = V.getLValuePath(); Entries.insert(Entries.end(), VEntries.begin(), VEntries.end()); - if (V.getLValueBase()) { - bool IsArray = false; + if (auto Base = V.getLValueBase()) { + if (auto Decl = Base.dyn_cast<ValueDecl const*>()) + Base = cast<ValueDecl>(Decl->getMostRecentDecl()); + bool IsArray = false, IsUnsizedArray = false; MostDerivedPathLength = findMostDerivedSubobject( - Ctx, V.getLValueBase(), V.getLValuePath(), MostDerivedArraySize, - MostDerivedType, IsArray); - MostDerivedIsArrayElement = IsArray; + Ctx, Base, V.getLValuePath(), MostDerivedArraySize, + MostDerivedType, IsArray, IsUnsizedArray); + MostDerivedIsArrayElement = IsArray; + MostDerivedIsAnUnsizedArray = IsUnsizedArray; } } } @@ -263,7 +271,7 @@ /// known bound. bool isMostDerivedAnUnsizedArray() const { assert(!Invalid && "Calling this makes no sense on invalid designators"); - return Entries.size() == 1 && FirstEntryIsAnUnsizedArray; + return MostDerivedIsAnUnsizedArray; } /// Determine what the most derived array's size is. Results in an assertion @@ -303,6 +311,7 @@ // This is a most-derived object. MostDerivedType = CAT->getElementType(); MostDerivedIsArrayElement = true; + MostDerivedIsAnUnsizedArray = false; MostDerivedArraySize = CAT->getSize().getZExtValue(); MostDerivedPathLength = Entries.size(); } @@ -315,6 +324,7 @@ MostDerivedType = ElemTy; MostDerivedIsArrayElement = true; + MostDerivedIsAnUnsizedArray = true; // 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. @@ -333,6 +343,7 @@ if (const FieldDecl *FD = dyn_cast<FieldDecl>(D)) { MostDerivedType = FD->getType(); MostDerivedIsArrayElement = false; + MostDerivedIsAnUnsizedArray = false; MostDerivedArraySize = 0; MostDerivedPathLength = Entries.size(); } @@ -347,53 +358,14 @@ // is unlikely to matter. MostDerivedType = EltTy; MostDerivedIsArrayElement = true; + MostDerivedIsAnUnsizedArray = false; MostDerivedArraySize = 2; MostDerivedPathLength = Entries.size(); } void diagnosePointerArithmetic(EvalInfo &Info, const Expr *E, const APSInt &N); /// Add N to the address of this subobject. - void adjustIndex(EvalInfo &Info, const Expr *E, APSInt N) { - if (Invalid || !N) return; - uint64_t TruncatedN = N.extOrTrunc(64).getZExtValue(); - if (isMostDerivedAnUnsizedArray()) { - // 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? - Entries.back().ArrayIndex += TruncatedN; - return; - } - - // [expr.add]p4: For the purposes of these operators, a pointer to a - // nonarray object behaves the same as a pointer to the first element of - // an array of length one with the type of the object as its element type. - bool IsArray = MostDerivedPathLength == Entries.size() && - MostDerivedIsArrayElement; - uint64_t ArrayIndex = - IsArray ? Entries.back().ArrayIndex : (uint64_t)IsOnePastTheEnd; - uint64_t ArraySize = - IsArray ? getMostDerivedArraySize() : (uint64_t)1; - - if (N < -(int64_t)ArrayIndex || N > ArraySize - ArrayIndex) { - // Calculate the actual index in a wide enough type, so we can include - // it in the note. - N = N.extend(std::max<unsigned>(N.getBitWidth() + 1, 65)); - (llvm::APInt&)N += ArrayIndex; - assert(N.ugt(ArraySize) && "bounds check failed for in-bounds index"); - diagnosePointerArithmetic(Info, E, N); - setInvalid(); - return; - } - - ArrayIndex += TruncatedN; - assert(ArrayIndex <= ArraySize && - "bounds check succeeded for out-of-bounds index"); - - if (IsArray) - Entries.back().ArrayIndex = ArrayIndex; - else - IsOnePastTheEnd = (ArrayIndex != 0); - } + void adjustIndex(EvalInfo &Info, const Expr *E, APSInt N); }; /// A stack frame in the constexpr call stack. @@ -495,7 +467,7 @@ // FIXME: Force the precision of the source value down so we don't // print digits which are usually useless (we don't really care here if // we truncate a digit by accident in edge cases). Ideally, - // APFloat::toString would automatically print the shortest + // APFloat::toString would automatically print the shortest // representation which rounds to the correct value, but it's a bit // tricky to implement. unsigned precision = @@ -720,7 +692,7 @@ private: OptionalDiagnostic Diag(SourceLocation Loc, diag::kind DiagId, unsigned ExtraNotes, bool IsCCEDiag) { - + if (EvalStatus.Diag) { // If we have a prior diagnostic, it will be noting that the expression // isn't a constant expression. This diagnostic is more important, @@ -773,7 +745,7 @@ unsigned ExtraNotes = 0) { return Diag(Loc, DiagId, ExtraNotes, false); } - + OptionalDiagnostic FFDiag(const Expr *E, diag::kind DiagId = diag::note_invalid_subexpr_in_const_expr, unsigned ExtraNotes = 0) { @@ -1086,6 +1058,53 @@ setInvalid(); } +void SubobjectDesignator::adjustIndex(EvalInfo &Info, const Expr *E, APSInt N) { + if (Invalid || !N) return; + + uint64_t TruncatedN = N.extOrTrunc(64).getZExtValue(); + if (isMostDerivedAnUnsizedArray()) { + // If we're dealing with an array without constant bound, the expression is + // not a constant expression. + if (!Info.checkingPotentialConstantExpression()) + Info.CCEDiag(E, diag::note_constexpr_array_unknown_bound_arithmetic); + // 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? + Entries.back().ArrayIndex += TruncatedN; + return; + } + + // [expr.add]p4: For the purposes of these operators, a pointer to a + // nonarray object behaves the same as a pointer to the first element of + // an array of length one with the type of the object as its element type. + bool IsArray = MostDerivedPathLength == Entries.size() && + MostDerivedIsArrayElement; + uint64_t ArrayIndex = + IsArray ? Entries.back().ArrayIndex : (uint64_t)IsOnePastTheEnd; + uint64_t ArraySize = + IsArray ? getMostDerivedArraySize() : (uint64_t)1; + + if (N < -(int64_t)ArrayIndex || N > ArraySize - ArrayIndex) { + // Calculate the actual index in a wide enough type, so we can include + // it in the note. + N = N.extend(std::max<unsigned>(N.getBitWidth() + 1, 65)); + (llvm::APInt&)N += ArrayIndex; + assert(N.ugt(ArraySize) && "bounds check failed for in-bounds index"); + diagnosePointerArithmetic(Info, E, N); + setInvalid(); + return; + } + + ArrayIndex += TruncatedN; + assert(ArrayIndex <= ArraySize && + "bounds check succeeded for out-of-bounds index"); + + if (IsArray) + Entries.back().ArrayIndex = ArrayIndex; + else + IsOnePastTheEnd = (ArrayIndex != 0); +} + CallStackFrame::CallStackFrame(EvalInfo &Info, SourceLocation CallLoc, const FunctionDecl *Callee, const LValue *This, APValue *Arguments) @@ -1214,8 +1233,6 @@ 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); } @@ -1281,10 +1298,6 @@ 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"); - Designator.FirstEntryIsAnUnsizedArray = true; Designator.addUnsizedArrayUnchecked(ElemTy); } void addArray(EvalInfo &Info, const Expr *E, const ConstantArrayType *CAT) { @@ -2569,7 +2582,7 @@ /// Find the designated sub-object of an rvalue. template<typename SubobjectHandler> typename SubobjectHandler::result_type -findSubobject(EvalInfo &Info, const Expr *E, const CompleteObject &Obj, +arc(EvalInfo &Info, const Expr *E, const CompleteObject &Obj, const SubobjectDesignator &Sub, SubobjectHandler &handler) { if (Sub.Invalid) // A diagnostic will have already been produced. @@ -3016,6 +3029,15 @@ if (!evaluateVarDeclInit(Info, E, VD, Frame, BaseVal)) return CompleteObject(); + + // The complete object can be an array of unknown bound, in which case we + // have to find the most recent declaration and adjust the type accordingly. + if (Info.Ctx.getAsIncompleteArrayType(BaseType)) { + QualType MostRecentType = + cast<ValueDecl const>(D->getMostRecentDecl())->getType(); + if (auto CAT = Info.Ctx.getAsConstantArrayType(MostRecentType)) + BaseType = MostRecentType; + } } else { const Expr *Base = LVal.Base.dyn_cast<const Expr*>(); @@ -4098,13 +4120,13 @@ if (Info.getLangOpts().CPlusPlus11) { const FunctionDecl *DiagDecl = Definition ? Definition : Declaration; - + // If this function is not constexpr because it is an inherited // non-constexpr constructor, diagnose that directly. auto *CD = dyn_cast<CXXConstructorDecl>(DiagDecl); if (CD && CD->isInheritingConstructor()) { auto *Inherited = CD->getInheritedConstructor().getConstructor(); - if (!Inherited->isConstexpr()) + if (!Inherited->isConstexpr()) DiagDecl = CD = Inherited; } @@ -4635,7 +4657,7 @@ return false; This = &ThisVal; Args = Args.slice(1); - } else if (MD && MD->isLambdaStaticInvoker()) { + } else if (MD && MD->isLambdaStaticInvoker()) { // Map the static invoker for the lambda back to the call operator. // Conveniently, we don't have to slice out the 'this' argument (as is // being done for the non-static case), since a static member function @@ -4670,7 +4692,7 @@ FD = LambdaCallOp; } - + } else return Error(E); @@ -5501,7 +5523,7 @@ // Update 'Result' to refer to the data member/field of the closure object // that represents the '*this' capture. if (!HandleLValueMember(Info, E, Result, - Info.CurrentCall->LambdaThisCaptureField)) + Info.CurrentCall->LambdaThisCaptureField)) return false; // If we captured '*this' by reference, replace the field with its referent. if (Info.CurrentCall->LambdaThisCaptureField->getType() @@ -5642,12 +5664,18 @@ Info, Result, SubExpr)) return false; } + // The result is a pointer to the first element of the array. if (const ConstantArrayType *CAT = Info.Ctx.getAsConstantArrayType(SubExpr->getType())) Result.addArray(Info, E, CAT); - else - Result.Designator.setInvalid(); + // If the array hasn't been given a bound yet, add it as an unsized one. + else { + auto AT = Info.Ctx.getAsArrayType(SubExpr->getType()); + assert(AT && "Array to pointer decay on non-array object?"); + Result.addUnsizedArray(Info, AT->getElementType()); + } + return true; case CK_FunctionToPointerDecay: @@ -6345,7 +6373,7 @@ if (ClosureClass->isInvalidDecl()) return false; if (Info.checkingPotentialConstantExpression()) return true; - + const size_t NumFields = std::distance(ClosureClass->field_begin(), ClosureClass->field_end()); @@ -6364,7 +6392,7 @@ assert(CaptureInitIt != E->capture_init_end()); // Get the initializer for this field Expr *const CurFieldInit = *CaptureInitIt++; - + // If there is no initializer, either this is a VLA or an error has // occurred. if (!CurFieldInit) @@ -6565,18 +6593,18 @@ // The number of initializers can be less than the number of // vector elements. For OpenCL, this can be due to nested vector - // initialization. For GCC compatibility, missing trailing elements + // initialization. For GCC compatibility, missing trailing elements // should be initialized with zeroes. unsigned CountInits = 0, CountElts = 0; while (CountElts < NumElements) { // Handle nested vector initialization. - if (CountInits < NumInits + if (CountInits < NumInits && E->getInit(CountInits)->getType()->isVectorType()) { APValue v; if (!EvaluateVector(E->getInit(CountInits), v, Info)) return Error(E); unsigned vlen = v.getVectorLength(); - for (unsigned j = 0; j < vlen; j++) + for (unsigned j = 0; j < vlen; j++) Elements.push_back(v.getVectorElt(j)); CountElts += vlen; } else if (EltTy->isIntegerType()) { @@ -6852,7 +6880,7 @@ } bool Success(const llvm::APInt &I, const Expr *E, APValue &Result) { - assert(E->getType()->isIntegralOrEnumerationType() && + assert(E->getType()->isIntegralOrEnumerationType() && "Invalid evaluation result."); assert(I.getBitWidth() == Info.Ctx.getIntWidth(E->getType()) && "Invalid evaluation result."); @@ -6866,7 +6894,7 @@ } bool Success(uint64_t Value, const Expr *E, APValue &Result) { - assert(E->getType()->isIntegralOrEnumerationType() && + assert(E->getType()->isIntegralOrEnumerationType() && "Invalid evaluation result."); Result = APValue(Info.Ctx.MakeIntValue(Value, E->getType())); return true; @@ -6942,7 +6970,7 @@ } return Success(Info.ArrayInitIndex, E); } - + // Note, GNU defines __null as an integer, not a pointer. bool VisitGNUNullExpr(const GNUNullExpr *E) { return ZeroInitialization(E); @@ -7306,10 +7334,8 @@ 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 this is an alloc_size base, we should ignore the initial array index + if (isBaseAnAllocSizeCall(Base)) { ++I; BaseType = BaseType->castAs<PointerType>()->getPointeeType(); } @@ -8096,12 +8122,12 @@ Result = RHSResult.Val; return true; } - + if (E->isLogicalOp()) { bool lhsResult, rhsResult; bool LHSIsOK = HandleConversionToBool(LHSResult.Val, lhsResult); bool RHSIsOK = HandleConversionToBool(RHSResult.Val, rhsResult); - + if (LHSIsOK) { if (RHSIsOK) { if (E->getOpcode() == BO_LOr) @@ -8117,34 +8143,34 @@ return Success(rhsResult, E, Result); } } - + return false; } - + assert(E->getLHS()->getType()->isIntegralOrEnumerationType() && E->getRHS()->getType()->isIntegralOrEnumerationType()); - + if (LHSResult.Failed || RHSResult.Failed) return false; - + const APValue &LHSVal = LHSResult.Val; const APValue &RHSVal = RHSResult.Val; - + // Handle cases like (unsigned long)&a + 4. if (E->isAdditiveOp() && LHSVal.isLValue() && RHSVal.isInt()) { Result = LHSVal; addOrSubLValueAsInteger(Result, RHSVal.getInt(), E->getOpcode() == BO_Sub); return true; } - + // Handle cases like 4 + (unsigned long)&a if (E->getOpcode() == BO_Add && RHSVal.isLValue() && LHSVal.isInt()) { Result = RHSVal; addOrSubLValueAsInteger(Result, LHSVal.getInt(), /*IsSub*/false); return true; } - + if (E->getOpcode() == BO_Sub && LHSVal.isLValue() && RHSVal.isLValue()) { // Handle (intptr_t)&&A - (intptr_t)&&B. if (!LHSVal.getLValueOffset().isZero() || @@ -8183,7 +8209,7 @@ void DataRecursiveIntBinOpEvaluator::process(EvalResult &Result) { Job &job = Queue.back(); - + switch (job.Kind) { case Job::AnyExprKind: { if (const BinaryOperator *Bop = dyn_cast<BinaryOperator>(job.E)) { @@ -8193,12 +8219,12 @@ return; } } - + EvaluateExpr(job.E, Result); Queue.pop_back(); return; } - + case Job::BinOpKind: { const BinaryOperator *Bop = cast<BinaryOperator>(job.E); bool SuppressRHSDiags = false; @@ -8213,7 +8239,7 @@ enqueue(Bop->getRHS()); return; } - + case Job::BinOpVisitedLHSKind: { const BinaryOperator *Bop = cast<BinaryOperator>(job.E); EvalResult RHS; @@ -8223,7 +8249,7 @@ return; } } - + llvm_unreachable("Invalid Job::Kind!"); } @@ -8735,7 +8761,7 @@ const RecordType *BaseRT = CurrentType->getAs<RecordType>(); if (!BaseRT) return Error(OOE); - + // Add the offset to the base. Result += RL.getBaseClassOffset(cast<CXXRecordDecl>(BaseRT->getDecl())); break; @@ -9929,7 +9955,7 @@ IsConst = false; return true; } - + // FIXME: Evaluating values of large array and record types can cause // performance problems. Only do so in C++11 for now. if (Exp->isRValue() && (Exp->getType()->isArrayType() || @@ -9951,7 +9977,7 @@ bool IsConst; if (FastEvaluateAsRValue(this, Result, Ctx, IsConst)) return IsConst; - + EvalInfo Info(Ctx, Result, EvalInfo::EM_IgnoreSideEffects); return ::EvaluateAsRValue(Info, this, Result.Val); } Index: include/clang/Basic/DiagnosticASTKinds.td =================================================================== --- include/clang/Basic/DiagnosticASTKinds.td +++ include/clang/Basic/DiagnosticASTKinds.td @@ -154,12 +154,14 @@ 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_array_unknown_bound_arithmetic : Note< + "cannot perform pointer arithmetic on pointer to array without constant bound">; def warn_integer_constant_overflow : Warning< "overflow in expression; result is %0 with type %1">, InGroup<DiagGroup<"integer-overflow">>; -// This is a temporary diagnostic, and shall be removed once our +// This is a temporary diagnostic, and shall be removed once our // implementation is complete, and like the preceding constexpr notes belongs // in Sema. def note_unimplemented_constexpr_lambda_feature_ast : Note<
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits