[PATCH] D123649: Allow flexible array initialization in C++.

aaron.ballman added a comment. In D123649#4362756 , @efriedma wrote: > In D123649#4362418 , @aaron.ballman > wrote: > >> In D123649#4362402 , @efriedma >> wrote: >> >>>

[PATCH] D123649: Allow flexible array initialization in C++.

efriedma added a comment. In D123649#4362418 , @aaron.ballman wrote: > In D123649#4362402 , @efriedma > wrote: > >> The assertion is correct; without it, your code is getting miscompiled. > > The assertion may b

[PATCH] D123649: Allow flexible array initialization in C++.

ahatanak added a comment. Looks like `Init`'s type is wrong. It should be `{ i32, [1 x i32] }` instead of `{ i32, [0 x i32] }` in the case where `S` has two members. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D123649/new/ https://reviews.llvm.or

[PATCH] D123649: Allow flexible array initialization in C++.

aaron.ballman added a comment. In D123649#4362402 , @efriedma wrote: > The assertion is correct; without it, your code is getting miscompiled. The assertion may be correct in its intent, but we still should not be asserting in practice: https://godbolt.

[PATCH] D123649: Allow flexible array initialization in C++.

efriedma added a comment. The assertion is correct; without it, your code is getting miscompiled. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D123649/new/ https://reviews.llvm.org/D123649 ___ cfe-commit

[PATCH] D123649: Allow flexible array initialization in C++.

ahatanak added a comment. The assertion also fails when the following code is compiled: struct S { int i[]; }; S value{0}; According to C99, flexible array members have to belong to structs that have more than one named member. But clang allows that as a GNU extension although gcc seems to

[PATCH] D123649: Allow flexible array initialization in C++.

ahatanak added a comment. We found another case where the assertion fails. $ cat test.cpp struct S { int len; int i[]; }; S value{0, 0}; $ clang++ -std=c++11 -c test.cpp `CstSize` is only 4 while `VarSize` is 8. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTIO

[PATCH] D123649: Allow flexible array initialization in C++.

efriedma added inline comments. Comment at: clang/lib/CodeGen/CodeGenModule.cpp:4639 + getDataLayout().getTypeAllocSize(Init->getType())); + assert(VarSize == CstSize && "Emitted constant has unexpected size"); +#endif ahatanak wrote: > efriedma wro

[PATCH] D123649: Allow flexible array initialization in C++.

ahatanak added inline comments. Comment at: clang/lib/CodeGen/CodeGenModule.cpp:4639 + getDataLayout().getTypeAllocSize(Init->getType())); + assert(VarSize == CstSize && "Emitted constant has unexpected size"); +#endif efriedma wrote: > ahatanak wro

[PATCH] D123649: Allow flexible array initialization in C++.

efriedma added inline comments. Comment at: clang/lib/CodeGen/CodeGenModule.cpp:4639 + getDataLayout().getTypeAllocSize(Init->getType())); + assert(VarSize == CstSize && "Emitted constant has unexpected size"); +#endif ahatanak wrote: > This asserti

[PATCH] D123649: Allow flexible array initialization in C++.

ahatanak added inline comments. Comment at: clang/lib/CodeGen/CodeGenModule.cpp:4639 + getDataLayout().getTypeAllocSize(Init->getType())); + assert(VarSize == CstSize && "Emitted constant has unexpected size"); +#endif This assertion fails when the

[PATCH] D123649: Allow flexible array initialization in C++.

efriedma added a comment. 6cf0b1b3 was a temporary fix to stop the crash. D123826 is the full fix to make the assertion actually work correctly. Repository: rG LLVM Github Monorepo CHAN

[PATCH] D123649: Allow flexible array initialization in C++.

nickdesaulniers added a comment. For posterity, posting the link to the failure this caused in Linaro's TCWG's CI of kernel builds w/ clang: https://lore.kernel.org/llvm/906914372.14298.1650022522881@jenkins.jenkins/. I'm guessing D123826 fixes this? Reposit

[PATCH] D123649: Allow flexible array initialization in C++.

efriedma added a comment. Apparently the assertions I added are triggering in the LLVM test-suite. Commented them out in 6cf0b1b3da3e8662baf030a2d741e3becaaab2b0 ; I'll come up with a proper fix soon. Repository: rG LLVM

[PATCH] D123649: Allow flexible array initialization in C++.

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG5955a0f9375a: Allow flexible array initialization in C++. (authored by efriedma). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D123649: Allow flexible array initialization in C++.

efriedma updated this revision to Diff 422671. efriedma added a comment. Address review comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D123649/new/ https://reviews.llvm.org/D123649 Files: clang/include/clang/AST/Decl.h clang/include/cla

[PATCH] D123649: Allow flexible array initialization in C++.

efriedma added inline comments. Comment at: clang/test/SemaCXX/constant-expression-cxx11.cpp:2388 + // evaluation. Make sure we emit a sane error message, for now. + constexpr A c = {1, 2, 3}; // expected-warning {{flexible array initialization is a GNU extension}} + static_a

[PATCH] D123649: Allow flexible array initialization in C++.

erichkeane added inline comments. Comment at: clang/lib/CodeGen/CGDecl.cpp:346 +else if (!D.getFlexibleArrayInitChars(getContext()).isZero()) + CGM.ErrorUnsupported(D.getInit(), "flexible array init"); else if (HaveInsertPoint()) { Can you write a t

[PATCH] D123649: Allow flexible array initialization in C++.

efriedma created this revision. efriedma added reviewers: aaron.ballman, rsmith, erichkeane. Herald added a project: All. efriedma requested review of this revision. Herald added a project: clang. Flexible array initialization is a C/C++ extension implemented in many compilers to allow initializi