[PATCH] D128248: [clang] Avoid an assertion in APValue::hasArrayFiller()

tbaeder abandoned this revision. tbaeder added a comment. Abandoning as per the discussion in https://discourse.llvm.org/t/apvalue-lifetime-problem-when-evaluating-constant-expressions/64002/5. This patch just hides a problem. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128248/new/

[PATCH] D128248: [clang] Avoid an assertion in APValue::hasArrayFiller()

tbaeder marked 2 inline comments as done. tbaeder added a comment. In D128248#3622775 , @shafik wrote: > LGTM after addressing all of Aaron's comments. I've already addressed them; did you miss the update or are you saying I haven't addressed them prope

[PATCH] D128248: [clang] Avoid an assertion in APValue::hasArrayFiller()

shafik accepted this revision. shafik added a comment. LGTM after addressing all of Aaron's comments. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128248/new/ https://reviews.llvm.org/D128248 ___ cfe-commits mailing list cfe-commits@lists.ll

[PATCH] D128248: [clang] Avoid an assertion in APValue::hasArrayFiller()

tbaeder updated this revision to Diff 441379. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128248/new/ https://reviews.llvm.org/D128248 Files: clang/docs/ReleaseNotes.rst clang/lib/AST/ExprConstant.cpp clang/test/SemaCXX/constexpr-array-init.cpp Index: clang/test/SemaCXX/constex

[PATCH] D128248: [clang] Avoid an assertion in APValue::hasArrayFiller()

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. Basically LGTM -- can you also add a release note about the assertion fix? Comment at: clang/test/SemaCXX/constexpr-array-init.cpp:1 + +// RUN: %clang_cc1 -std=

[PATCH] D128248: [clang] Avoid an assertion in APValue::hasArrayFiller()

tbaeder added a comment. I added the `isArray()` check in both places now, but the first one is not necessary for the test case (at least). CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128248/new/ https://reviews.llvm.org/D128248 ___ cfe-co

[PATCH] D128248: [clang] Avoid an assertion in APValue::hasArrayFiller()

tbaeder updated this revision to Diff 441344. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128248/new/ https://reviews.llvm.org/D128248 Files: clang/lib/AST/ExprConstant.cpp clang/test/SemaCXX/constexpr-array-init.cpp Index: clang/test/SemaCXX/constexpr-array-init.cpp

[PATCH] D128248: [clang] Avoid an assertion in APValue::hasArrayFiller()

shafik added inline comments. Comment at: clang/include/clang/AST/APValue.h:511-512 bool hasArrayFiller() const { +if (!isArray()) + return false; return getArrayInitializedElts() != getArraySize(); aaron.ballman wrote: > I think this makes the i

[PATCH] D128248: [clang] Avoid an assertion in APValue::hasArrayFiller()

shafik added a comment. > and run the test case, the first added line prints `1` and the second one > `0`. `Result` is being mutated when doing the in-place evaluation. I did not catch that. That is unfortunate, I am wondering now if we need a `Result.isArray() && ` before the `EvaluateInPlace`

[PATCH] D128248: [clang] Avoid an assertion in APValue::hasArrayFiller()

tbaeder added a comment. If you apply e.g. @@ -10739,6 +10779,7 @@ bool ArrayExprEvaluator::VisitInitListExpr(const InitListExpr *E, << NumEltsToInit << ".\n"); Result = APValue(APValue::UninitArray(), NumEltsToInit, NumElts); + llvm::errs() << "Result "

[PATCH] D128248: [clang] Avoid an assertion in APValue::hasArrayFiller()

shafik added a comment. Are you sure it is failing on the line you indicated? After looking at the code, a little before that line we have: Result = APValue(APValue::UninitArray(), NumEltsToInit, NumElts); which should assure that we have an array at this point. Looking at the result of the

[PATCH] D128248: [clang] Avoid an assertion in APValue::hasArrayFiller()

aaron.ballman added inline comments. Comment at: clang/include/clang/AST/APValue.h:511-512 bool hasArrayFiller() const { +if (!isArray()) + return false; return getArrayInitializedElts() != getArraySize(); I think this makes the interface somewha

[PATCH] D128248: [clang] Avoid an assertion in APValue::hasArrayFiller()

tbaeder added a comment. Failing tests are just because the test file is not formatted it seems. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128248/new/ https://reviews.llvm.org/D128248 ___ cfe-commits mailing list cfe-commits@lists.llvm.or

[PATCH] D128248: [clang] Avoid an assertion in APValue::hasArrayFiller()

tbaeder updated this revision to Diff 438612. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128248/new/ https://reviews.llvm.org/D128248 Files: clang/include/clang/AST/APValue.h clang/test/SemaCXX/constexpr-array-init.cpp Index: clang/test/SemaCXX/constexpr-array-init.cpp =

[PATCH] D128248: [clang] Avoid an assertion in APValue::hasArrayFiller()

tbaeder created this revision. tbaeder added reviewers: aaron.ballman, rsmith. Herald added a project: All. tbaeder requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. When calling `hasArrayFiller()` on a `APValue` that's not an array, the two