[PATCH] D124038: [clang] Prevent folding of non-const compound expr

2022-05-15 Thread serge via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG924acb624f58: [clang] Prevent folding of non-const compound expr (authored by serge-sans-paille). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124038/new/

[PATCH] D124038: [clang] Prevent folding of non-const compound expr

2022-05-13 Thread Eli Friedman via Phabricator via cfe-commits
efriedma accepted this revision. efriedma added a comment. This revision is now accepted and ready to land. LGTM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124038/new/ https://reviews.llvm.org/D124038 ___ cfe-commits mailing list

[PATCH] D124038: [clang] Prevent folding of non-const compound expr

2022-05-13 Thread serge via Phabricator via cfe-commits
serge-sans-paille updated this revision to Diff 429156. serge-sans-paille added a comment. Update messed up format CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124038/new/ https://reviews.llvm.org/D124038 Files: clang/lib/AST/ExprConstant.cpp

[PATCH] D124038: [clang] Prevent folding of non-const compound expr

2022-05-12 Thread Eli Friedman via Phabricator via cfe-commits
efriedma added a comment. Updated messed up formatting? Otherwise, I guess this is fine. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124038/new/ https://reviews.llvm.org/D124038 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D124038: [clang] Prevent folding of non-const compound expr

2022-05-10 Thread serge via Phabricator via cfe-commits
serge-sans-paille updated this revision to Diff 428562. serge-sans-paille added a comment. Update GCC manual quote CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124038/new/ https://reviews.llvm.org/D124038 Files: clang/lib/AST/ExprConstant.cpp

[PATCH] D124038: [clang] Prevent folding of non-const compound expr

2022-05-10 Thread Eli Friedman via Phabricator via cfe-commits
efriedma added a comment. I think so, yes. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124038/new/ https://reviews.llvm.org/D124038 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D124038: [clang] Prevent folding of non-const compound expr

2022-05-10 Thread serge via Phabricator via cfe-commits
serge-sans-paille added a comment. In D124038#3504371 , @efriedma wrote: > I think you're looking at old documentation? Here's what the current page > (https://gcc.gnu.org/onlinedocs/gcc/Compound-Literals.html) has to say: Indeed! I was looking at my

[PATCH] D124038: [clang] Prevent folding of non-const compound expr

2022-05-10 Thread Eli Friedman via Phabricator via cfe-commits
efriedma added a comment. I think you're looking at old documentation? Here's what the current page (https://gcc.gnu.org/onlinedocs/gcc/Compound-Literals.html) has to say: In C, a compound literal designates an unnamed object with static or automatic storage duration. In C++, a compound

[PATCH] D124038: [clang] Prevent folding of non-const compound expr

2022-05-10 Thread serge via Phabricator via cfe-commits
serge-sans-paille added inline comments. Comment at: clang/test/SemaCXX/constant-expression-cxx11.cpp:1609 struct X { int a[2]; }; constexpr int *n = (X){1, 2}.a; // expected-warning {{C99}} expected-warning {{temporary}} // expected-error@-1 {{constant expression}}

[PATCH] D124038: [clang] Prevent folding of non-const compound expr

2022-05-10 Thread serge via Phabricator via cfe-commits
serge-sans-paille updated this revision to Diff 428317. serge-sans-paille edited the summary of this revision. serge-sans-paille added a comment. Added reg to GCC info page to explain current behavior, and make the test more explicit with respect to that quote. CHANGES SINCE LAST ACTION

[PATCH] D124038: [clang] Prevent folding of non-const compound expr

2022-05-09 Thread Eli Friedman via Phabricator via cfe-commits
efriedma added inline comments. Comment at: clang/lib/AST/ExprConstant.cpp:4267 + bool IsConstant = CLETy.isConstant(Info.Ctx); + if (!IsConstant && CLETy->isArrayType()) { +Info.FFDiag(Conv); serge-sans-paille wrote: > efriedma wrote: > > Is

[PATCH] D124038: [clang] Prevent folding of non-const compound expr

2022-05-09 Thread serge via Phabricator via cfe-commits
serge-sans-paille added inline comments. Comment at: clang/lib/AST/ExprConstant.cpp:4267 + bool IsConstant = CLETy.isConstant(Info.Ctx); + if (!IsConstant && CLETy->isArrayType()) { +Info.FFDiag(Conv); efriedma wrote: > Is the "isArrayType()"

[PATCH] D124038: [clang] Prevent folding of non-const compound expr

2022-05-09 Thread Eli Friedman via Phabricator via cfe-commits
efriedma added inline comments. Comment at: clang/lib/AST/ExprConstant.cpp:4267 + bool IsConstant = CLETy.isConstant(Info.Ctx); + if (!IsConstant && CLETy->isArrayType()) { +Info.FFDiag(Conv); Is the "isArrayType()" check here necessary?

[PATCH] D124038: [clang] Prevent folding of non-const compound expr

2022-05-09 Thread serge via Phabricator via cfe-commits
serge-sans-paille updated this revision to Diff 428042. serge-sans-paille added a comment. Match GCC behavior here: some test case were previously accepted while having the opposite behavior. This pacth both fixes the original issue and adopt gcc behavior. CHANGES SINCE LAST ACTION

[PATCH] D124038: [clang] Prevent folding of non-const compound expr

2022-04-20 Thread Eli Friedman via Phabricator via cfe-commits
efriedma added a comment. The `VD->getAnyInitializer()->IgnoreCasts()` is the part that's wrong. We want to allow something like the following: constexpr int *q = (int *)(int[1]){3}; constexpr int *q2 = q; To catch the bad cases specifically, we have to wait until we actually reach the

[PATCH] D124038: [clang] Prevent folding of non-const compound expr

2022-04-20 Thread serge via Phabricator via cfe-commits
serge-sans-paille added a comment. Alternatively, the following also works, but it splits the logic into anotherplace, while current patch is at least consistent with existing state diff --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp index 498f0d4..233307f 100644

[PATCH] D124038: [clang] Prevent folding of non-const compound expr

2022-04-20 Thread serge via Phabricator via cfe-commits
serge-sans-paille added a comment. In D124038#3460731 , @efriedma wrote: > The fix doesn't look right. > > A CompoundLiteralExpr is itself an lvalue; we should catch any issues when we > visit it, not a VarDecl that contains a pointer to it. Well, the

[PATCH] D124038: [clang] Prevent folding of non-const compound expr

2022-04-19 Thread Eli Friedman via Phabricator via cfe-commits
efriedma added a comment. The fix doesn't look right. A CompoundLiteralExpr is itself an lvalue; we should catch any issues when we visit it, not a VarDecl that contains a pointer to it. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124038/new/

[PATCH] D124038: [clang] Prevent folding of non-const compound expr

2022-04-19 Thread serge via Phabricator via cfe-commits
serge-sans-paille created this revision. serge-sans-paille added a reviewer: aaron.ballman. Herald added a project: All. serge-sans-paille requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. When a non-const compound statement is used to