This revision was automatically updated to reflect the committed changes.
Closed by commit rL268018: Implementation of VlA of GNU C++ extension, by
Vladimir Yakovlev. (authored by ABataev).
Changed prior to commit:
http://reviews.llvm.org/D18823?vs=53658=4#toc
Repository:
rL LLVM
rsmith accepted this revision.
rsmith added a comment.
This revision is now accepted and ready to land.
LGTM.
http://reviews.llvm.org/D18823
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
vbyakovl added a comment.
@rsmith ping
http://reviews.llvm.org/D18823
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
vbyakovl added a comment.
Richard, now my changes are good for you?
http://reviews.llvm.org/D18823
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
vbyakovl updated this revision to Diff 53658.
vbyakovl added a comment.
Sema/SemaType.cpp: Deleted POD type and default constructor checkings.
test/SemaCXX/c99-variable-length-array-cxx11.cpp: Edited dianocnics.
http://reviews.llvm.org/D18823
Files:
llvm/tools/clang/lib/CodeGen/CGClass.cpp
rsmith added inline comments.
Comment at: llvm/tools/clang/lib/Sema/SemaType.cpp:2163-2165
@@ -2161,3 +2162,5 @@
if (!T->isDependentType() && isCompleteType(Loc, BaseT) &&
- !BaseT.isPODType(Context) && !BaseT->isObjCLifetimeType()) {
+ RD &&
vbyakovl updated this revision to Diff 53548.
vbyakovl added a comment.
Sema/SemaType.cpp: Vla works in C++ by default. An error is printed if a class
has not default constructor.
test/SemaCXX/c99-variable-length-array.cpp: Changes is removed
test/SemaCXX/vla.cpp: changes is removed.
doug.gregor added a comment.
I think it's completely reasonable to implement support for VLAs as a GNU C++
extension. We did go through a phase where we tried to avoid implementing VLAs
in C++ because we considered them to be a poor feature in C++. However, their
use was wide-spread enough
rsmith added inline comments.
Comment at: llvm/tools/clang/lib/Sema/SemaType.cpp:2158-2159
@@ -2157,3 +2157,4 @@
if (!getLangOpts().C99) {
-if (T->isVariableArrayType()) {
+if (T->isVariableArrayType() &&
+!(getLangOpts().CPlusPlus && getLangOpts().GNUMode)) {
rjmccall added a comment.
It requires careful IRGen support which at one point did not exist. When I
implemented VLAs of __strong/__weak references for ARC, I deliberately tried to
make sure that the infrastructure would also work for the C++ cases, but I
didn't have time to go actually test
rsmith added a comment.
@vbyakovl The description of this patch is very misleading. What I believe this
does is to allow VLAs with non-trivially-destructed element types. Can you fix
the description (and make sure the fixed version ends up in the commit message
if this is approved)?
@rjmccall
hfinkel added a comment.
In http://reviews.llvm.org/D18823#397122, @rengolin wrote:
> In http://reviews.llvm.org/D18823#397112, @hfinkel wrote:
>
> > However, as an implementation extension, this concern is not relevant. I'm
> > in favor of this; I have uses who use this feature in GCC. It is
rengolin added a comment.
In http://reviews.llvm.org/D18823#397112, @hfinkel wrote:
> However, as an implementation extension, this concern is not relevant. I'm in
> favor of this; I have uses who use this feature in GCC. It is certainly true
> that most HPC users are using these on PODs, but
hfinkel added a subscriber: hfinkel.
hfinkel added a comment.
In http://reviews.llvm.org/D18823#397071, @rengolin wrote:
> En passant comment: I really wish we wouldn't.
>
> The C++ standard had some very careful considerations on VLAs and decided
> *not* to support. It wasn't for lack of
rengolin added a subscriber: rengolin.
rengolin added a comment.
En passant comment: I really wish we wouldn't.
The C++ standard had some very careful considerations on VLAs and decided *not*
to support. It wasn't for lack of interest, it was a well informed decision.
Comment
DmitryPolukhin added inline comments.
Comment at: llvm/tools/clang/test/SemaCXX/c99-variable-length-array.cpp:1
@@ -1,1 +1,2 @@
-// RUN: %clang_cc1 -fsyntax-only -verify -Wvla-extension %s
+// RUN: %clang_cc1 -fsyntax-only -std=c++98 -DCPP98 -verify -Wvla-extension %s
+// RUN:
aaron.ballman added reviewers: rsmith, doug.gregor.
aaron.ballman added a comment.
Adding a few more reviewers.
Is there a major use case driving this patch, or is this just for GCC
compatibility? Personally, I would prefer to avoid adding VLA support to C++
unless there's a good motivating
DmitryPolukhin added a comment.
The patch itself looks good to me but the main question do we want to support
VLA in C++ in GNU extension mode.
http://reviews.llvm.org/D18823
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
vbyakovl created this revision.
vbyakovl added a reviewer: aaron.s.wishnick.
vbyakovl added subscribers: DmitryPolukhin, cfe-commits.
This implements GNU C++ extension "Variable length array". This works under
-std=gnu++98.
http://reviews.llvm.org/D18823
Files:
19 matches
Mail list logo
