[PATCH] D49952: Check for NULL Destination-Type when creating ArrayConstant

[John McCall via Phabricator via cfe-commits]

rjmccall accepted this revision.
rjmccall added a comment.
This revision is now accepted and ready to land.

Sure, that's fine.


https://reviews.llvm.org/D49952



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D49952: Check for NULL Destination-Type when creating ArrayConstant

[Balaji Iyer via Phabricator via cfe-commits]

bviyer marked an inline comment as done.
bviyer added a comment.

John, I have updated the test case as you requested (I think). I am a bit new 
to Clang, so apologize if I mistook your request.


https://reviews.llvm.org/D49952



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D49952: Check for NULL Destination-Type when creating ArrayConstant

[Balaji Iyer via Phabricator via cfe-commits]

bviyer updated this revision to Diff 159353.

https://reviews.llvm.org/D49952

Files:
  lib/CodeGen/CGExprConstant.cpp
  test/CodeGenCXX/empty-struct-init-list.cpp


Index: test/CodeGenCXX/empty-struct-init-list.cpp
===
--- /dev/null
+++ test/CodeGenCXX/empty-struct-init-list.cpp
@@ -0,0 +1,12 @@
+// RUN: %clang_cc1 -std=c++11 -emit-llvm -o - %s | FileCheck %s
+// RUN: %clang_cc1 -std=c++14 -emit-llvm -o - %s | FileCheck %s
+// RUN: %clang_cc1 -std=c++17 -emit-llvm -o - %s | FileCheck %s
+
+// CHECK: struct.a
+typedef struct { } a;
+typedef struct {
+  a b[];
+} c;
+
+// CHECK: global %struct.c zeroinitializer, align 1
+c d{ };
Index: lib/CodeGen/CGExprConstant.cpp
===
--- lib/CodeGen/CGExprConstant.cpp
+++ lib/CodeGen/CGExprConstant.cpp
@@ -2119,6 +2119,16 @@
   Elts.push_back(C);
 }
 
+// This means that the array type is probably "IncompleteType" or some
+// type that is not ConstantArray.
+if (CAT == nullptr && CommonElementType == nullptr && !NumInitElts) {
+  const ArrayType *AT = CGM.getContext().getAsArrayType(DestType);
+  CommonElementType = CGM.getTypes().ConvertType(AT->getElementType());
+  llvm::ArrayType *AType = llvm::ArrayType::get(CommonElementType,
+NumElements);
+  return llvm::ConstantAggregateZero::get(AType);
+}
+
 return EmitArrayConstant(CGM, CAT, CommonElementType, NumElements, Elts,
  Filler);
   }


Index: test/CodeGenCXX/empty-struct-init-list.cpp
===
--- /dev/null
+++ test/CodeGenCXX/empty-struct-init-list.cpp
@@ -0,0 +1,12 @@
+// RUN: %clang_cc1 -std=c++11 -emit-llvm -o - %s | FileCheck %s
+// RUN: %clang_cc1 -std=c++14 -emit-llvm -o - %s | FileCheck %s
+// RUN: %clang_cc1 -std=c++17 -emit-llvm -o - %s | FileCheck %s
+
+// CHECK: struct.a
+typedef struct { } a;
+typedef struct {
+  a b[];
+} c;
+
+// CHECK: global %struct.c zeroinitializer, align 1
+c d{ };
Index: lib/CodeGen/CGExprConstant.cpp
===
--- lib/CodeGen/CGExprConstant.cpp
+++ lib/CodeGen/CGExprConstant.cpp
@@ -2119,6 +2119,16 @@
   Elts.push_back(C);
 }
 
+// This means that the array type is probably "IncompleteType" or some
+// type that is not ConstantArray.
+if (CAT == nullptr && CommonElementType == nullptr && !NumInitElts) {
+  const ArrayType *AT = CGM.getContext().getAsArrayType(DestType);
+  CommonElementType = CGM.getTypes().ConvertType(AT->getElementType());
+  llvm::ArrayType *AType = llvm::ArrayType::get(CommonElementType,
+NumElements);
+  return llvm::ConstantAggregateZero::get(AType);
+}
+
 return EmitArrayConstant(CGM, CAT, CommonElementType, NumElements, Elts,
  Filler);
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D49952: Check for NULL Destination-Type when creating ArrayConstant

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.



Comment at: lib/CodeGen/CGExprConstant.cpp:2123
+// This means that the array type is probably "IncompleteType" or some
+// type that is not ConstantArray.
+if (CAT == nullptr && CommonElementType == nullptr && !NumInitElts) {

It definitely means that it's an incomplete array type, i.e. a flexible array 
member.



Comment at: test/CodeGenCXX/empty-struct-init-list.cpp:11
+} c;
+c d{ };

rjmccall wrote:
> Please make this a `FileCheck` test that tests the actual compiler output.
I'm sorry, I should've been more specific.  Please include a test line that 
shows the complete constant generated to initialize `@d`.


Repository:
  rC Clang

https://reviews.llvm.org/D49952



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D49952: Check for NULL Destination-Type when creating ArrayConstant

[Balaji Iyer via Phabricator via cfe-commits]

bviyer updated this revision to Diff 158676.
bviyer added a comment.

Added FileCheck as requested by John McCall
Made comments start with "//" instead of /*
Removed expected-no-diagnostics from the test case (as requested by Erik)


Repository:
  rC Clang

https://reviews.llvm.org/D49952

Files:
  lib/CodeGen/CGExprConstant.cpp
  test/CodeGenCXX/empty-struct-init-list.cpp


Index: test/CodeGenCXX/empty-struct-init-list.cpp
===
--- /dev/null
+++ test/CodeGenCXX/empty-struct-init-list.cpp
@@ -0,0 +1,12 @@
+// RUN: %clang_cc1 -std=c++11 -emit-llvm -o - %s | FileCheck %s
+// RUN: %clang_cc1 -std=c++14 -emit-llvm -o - %s | FileCheck %s
+// RUN: %clang_cc1 -std=c++17 -emit-llvm -o - %s | FileCheck %s
+
+// CHECK: struct.a
+typedef struct { } a;
+typedef struct {
+  a b[];
+} c;
+
+// CHECK: struct.c
+c d{ };
Index: lib/CodeGen/CGExprConstant.cpp
===
--- lib/CodeGen/CGExprConstant.cpp
+++ lib/CodeGen/CGExprConstant.cpp
@@ -2119,6 +2119,16 @@
   Elts.push_back(C);
 }
 
+// This means that the array type is probably "IncompleteType" or some
+// type that is not ConstantArray.
+if (CAT == nullptr && CommonElementType == nullptr && !NumInitElts) {
+  const ArrayType *AT = CGM.getContext().getAsArrayType(DestType);
+  CommonElementType = CGM.getTypes().ConvertType(AT->getElementType());
+  llvm::ArrayType *AType = llvm::ArrayType::get(CommonElementType,
+NumElements);
+  return llvm::ConstantAggregateZero::get(AType);
+}
+
 return EmitArrayConstant(CGM, CAT, CommonElementType, NumElements, Elts,
  Filler);
   }


Index: test/CodeGenCXX/empty-struct-init-list.cpp
===
--- /dev/null
+++ test/CodeGenCXX/empty-struct-init-list.cpp
@@ -0,0 +1,12 @@
+// RUN: %clang_cc1 -std=c++11 -emit-llvm -o - %s | FileCheck %s
+// RUN: %clang_cc1 -std=c++14 -emit-llvm -o - %s | FileCheck %s
+// RUN: %clang_cc1 -std=c++17 -emit-llvm -o - %s | FileCheck %s
+
+// CHECK: struct.a
+typedef struct { } a;
+typedef struct {
+  a b[];
+} c;
+
+// CHECK: struct.c
+c d{ };
Index: lib/CodeGen/CGExprConstant.cpp
===
--- lib/CodeGen/CGExprConstant.cpp
+++ lib/CodeGen/CGExprConstant.cpp
@@ -2119,6 +2119,16 @@
   Elts.push_back(C);
 }
 
+// This means that the array type is probably "IncompleteType" or some
+// type that is not ConstantArray.
+if (CAT == nullptr && CommonElementType == nullptr && !NumInitElts) {
+  const ArrayType *AT = CGM.getContext().getAsArrayType(DestType);
+  CommonElementType = CGM.getTypes().ConvertType(AT->getElementType());
+  llvm::ArrayType *AType = llvm::ArrayType::get(CommonElementType,
+NumElements);
+  return llvm::ConstantAggregateZero::get(AType);
+}
+
 return EmitArrayConstant(CGM, CAT, CommonElementType, NumElements, Elts,
  Filler);
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D49952: Check for NULL Destination-Type when creating ArrayConstant

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.



Comment at: test/CodeGenCXX/empty-struct-init-list.cpp:11
+} c;
+c d{ };

Please make this a `FileCheck` test that tests the actual compiler output.


https://reviews.llvm.org/D49952



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D49952: Check for NULL Destination-Type when creating ArrayConstant

[Balaji Iyer via Phabricator via cfe-commits]

bviyer updated this revision to Diff 158392.
bviyer added a comment.

Forgot to pass the -emit-llvm to the test case.


https://reviews.llvm.org/D49952

Files:
  lib/CodeGen/CGExprConstant.cpp
  test/CodeGenCXX/empty-struct-init-list.cpp


Index: test/CodeGenCXX/empty-struct-init-list.cpp
===
--- /dev/null
+++ test/CodeGenCXX/empty-struct-init-list.cpp
@@ -0,0 +1,11 @@
+// RUN: %clang_cc1 -std=c++11 -emit-llvm %s
+// RUN: %clang_cc1 -std=c++14 -emit-llvm %s
+// RUN: %clang_cc1 -std=c++17 -emit-llvm %s
+// expected-no-diagnostics
+
+typedef struct { } a;
+
+typedef struct {
+  a b[];
+} c;
+c d{ };
Index: lib/CodeGen/CGExprConstant.cpp
===
--- lib/CodeGen/CGExprConstant.cpp
+++ lib/CodeGen/CGExprConstant.cpp
@@ -2119,6 +2119,16 @@
   Elts.push_back(C);
 }
 
+/* This means that the array type is probably "IncompleteType" or some
+   type that is not ConstantArray.  */
+if (CAT == nullptr && CommonElementType == nullptr && !NumInitElts) {
+  const ArrayType *AT = CGM.getContext().getAsArrayType(DestType);
+  CommonElementType = CGM.getTypes().ConvertType(AT->getElementType());
+  llvm::ArrayType *AType = llvm::ArrayType::get(CommonElementType,
+NumElements);
+  return llvm::ConstantAggregateZero::get(AType);
+}
+
 return EmitArrayConstant(CGM, CAT, CommonElementType, NumElements, Elts,
  Filler);
   }


Index: test/CodeGenCXX/empty-struct-init-list.cpp
===
--- /dev/null
+++ test/CodeGenCXX/empty-struct-init-list.cpp
@@ -0,0 +1,11 @@
+// RUN: %clang_cc1 -std=c++11 -emit-llvm %s
+// RUN: %clang_cc1 -std=c++14 -emit-llvm %s
+// RUN: %clang_cc1 -std=c++17 -emit-llvm %s
+// expected-no-diagnostics
+
+typedef struct { } a;
+
+typedef struct {
+  a b[];
+} c;
+c d{ };
Index: lib/CodeGen/CGExprConstant.cpp
===
--- lib/CodeGen/CGExprConstant.cpp
+++ lib/CodeGen/CGExprConstant.cpp
@@ -2119,6 +2119,16 @@
   Elts.push_back(C);
 }
 
+/* This means that the array type is probably "IncompleteType" or some
+   type that is not ConstantArray.  */
+if (CAT == nullptr && CommonElementType == nullptr && !NumInitElts) {
+  const ArrayType *AT = CGM.getContext().getAsArrayType(DestType);
+  CommonElementType = CGM.getTypes().ConvertType(AT->getElementType());
+  llvm::ArrayType *AType = llvm::ArrayType::get(CommonElementType,
+NumElements);
+  return llvm::ConstantAggregateZero::get(AType);
+}
+
 return EmitArrayConstant(CGM, CAT, CommonElementType, NumElements, Elts,
  Filler);
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D49952: Check for NULL Destination-Type when creating ArrayConstant

[Balaji Iyer via Phabricator via cfe-commits]

bviyer updated this revision to Diff 158391.
bviyer added a comment.

Fixed code as suggested by John McCall.


Repository:
  rC Clang

https://reviews.llvm.org/D49952

Files:
  lib/CodeGen/CGExprConstant.cpp
  test/CodeGenCXX/empty-struct-init-list.cpp


Index: test/CodeGenCXX/empty-struct-init-list.cpp
===
--- /dev/null
+++ test/CodeGenCXX/empty-struct-init-list.cpp
@@ -0,0 +1,11 @@
+// RUN: %clang_cc1 -std=c++11  %s
+// RUN: %clang_cc1 -std=c++14  %s
+// RUN: %clang_cc1 -std=c++17  %s
+// expected-no-diagnostics
+
+typedef struct { } a;
+
+typedef struct {
+  a b[];
+} c;
+c d{ };
Index: lib/CodeGen/CGExprConstant.cpp
===
--- lib/CodeGen/CGExprConstant.cpp
+++ lib/CodeGen/CGExprConstant.cpp
@@ -2119,6 +2119,16 @@
   Elts.push_back(C);
 }
 
+/* This means that the array type is probably "IncompleteType" or some
+   type that is not ConstantArray.  */
+if (CAT == nullptr && CommonElementType == nullptr && !NumInitElts) {
+  const ArrayType *AT = CGM.getContext().getAsArrayType(DestType);
+  CommonElementType = CGM.getTypes().ConvertType(AT->getElementType());
+  llvm::ArrayType *AType = llvm::ArrayType::get(CommonElementType,
+NumElements);
+  return llvm::ConstantAggregateZero::get(AType);
+}
+
 return EmitArrayConstant(CGM, CAT, CommonElementType, NumElements, Elts,
  Filler);
   }


Index: test/CodeGenCXX/empty-struct-init-list.cpp
===
--- /dev/null
+++ test/CodeGenCXX/empty-struct-init-list.cpp
@@ -0,0 +1,11 @@
+// RUN: %clang_cc1 -std=c++11  %s
+// RUN: %clang_cc1 -std=c++14  %s
+// RUN: %clang_cc1 -std=c++17  %s
+// expected-no-diagnostics
+
+typedef struct { } a;
+
+typedef struct {
+  a b[];
+} c;
+c d{ };
Index: lib/CodeGen/CGExprConstant.cpp
===
--- lib/CodeGen/CGExprConstant.cpp
+++ lib/CodeGen/CGExprConstant.cpp
@@ -2119,6 +2119,16 @@
   Elts.push_back(C);
 }
 
+/* This means that the array type is probably "IncompleteType" or some
+   type that is not ConstantArray.  */
+if (CAT == nullptr && CommonElementType == nullptr && !NumInitElts) {
+  const ArrayType *AT = CGM.getContext().getAsArrayType(DestType);
+  CommonElementType = CGM.getTypes().ConvertType(AT->getElementType());
+  llvm::ArrayType *AType = llvm::ArrayType::get(CommonElementType,
+NumElements);
+  return llvm::ConstantAggregateZero::get(AType);
+}
+
 return EmitArrayConstant(CGM, CAT, CommonElementType, NumElements, Elts,
  Filler);
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D49952: Check for NULL Destination-Type when creating ArrayConstant

[Erik Pilkington via Phabricator via cfe-commits]

erik.pilkington added inline comments.



Comment at: test/CodeGenCXX/empty-struct-init-list.cpp:1-4
+// RUN: %clang_cc1 -std=c++11  %s
+// RUN: %clang_cc1 -std=c++14  %s
+// RUN: %clang_cc1 -std=c++17  %s
+// expected-no-diagnostics

You should add -emit-llvm, or else CodeGen won't even run. Also, 
expected-no-diagnostics only means anything when -verify is included on the run 
line.


Repository:
  rC Clang

https://reviews.llvm.org/D49952



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D49952: Check for NULL Destination-Type when creating ArrayConstant

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

I think the right fix here is to ensure that `DestType` is non-null at a higher 
level.  Older branches of the compiler seem to be able to correctly emit this, 
probably because the initializer generated for this field ends up having type 
`a[0]`.  Maybe we've just done some refactor that tries to optimize that but 
doesn't handle flexible array members correctly.


Repository:
  rC Clang

https://reviews.llvm.org/D49952



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D49952: Check for NULL Destination-Type when creating ArrayConstant

[Balaji Iyer via Phabricator via cfe-commits]

bviyer created this revision.
bviyer added reviewers: arphaman, dexonsmith, ahatanak, rjmccall.

While emitting Array Constant, if the destination type is null-pointer, it will 
cause an assert. This patch will check if the destination type is null, and if 
so then it will just return nullptr as the array constant (not something that 
is derived from destination type). 
A test case is also attached.


Repository:
  rC Clang

https://reviews.llvm.org/D49952

Files:
  lib/CodeGen/CGExprConstant.cpp
  test/CodeGenCXX/empty-struct-init-list.cpp


Index: test/CodeGenCXX/empty-struct-init-list.cpp
===
--- /dev/null
+++ test/CodeGenCXX/empty-struct-init-list.cpp
@@ -0,0 +1,11 @@
+// RUN: %clang_cc1 -std=c++11  %s
+// RUN: %clang_cc1 -std=c++14  %s
+// RUN: %clang_cc1 -std=c++17  %s
+// expected-no-diagnostics
+
+typedef struct { } a;
+
+typedef struct {
+  a b[];
+} c;
+c d{ };
Index: lib/CodeGen/CGExprConstant.cpp
===
--- lib/CodeGen/CGExprConstant.cpp
+++ lib/CodeGen/CGExprConstant.cpp
@@ -650,6 +650,8 @@
   }
 
   if (NonzeroLength == 0) {
+if (DestType == nullptr)
+  return nullptr;
 return llvm::ConstantAggregateZero::get(
 CGM.getTypes().ConvertType(QualType(DestType, 0)));
   }


Index: test/CodeGenCXX/empty-struct-init-list.cpp
===
--- /dev/null
+++ test/CodeGenCXX/empty-struct-init-list.cpp
@@ -0,0 +1,11 @@
+// RUN: %clang_cc1 -std=c++11  %s
+// RUN: %clang_cc1 -std=c++14  %s
+// RUN: %clang_cc1 -std=c++17  %s
+// expected-no-diagnostics
+
+typedef struct { } a;
+
+typedef struct {
+  a b[];
+} c;
+c d{ };
Index: lib/CodeGen/CGExprConstant.cpp
===
--- lib/CodeGen/CGExprConstant.cpp
+++ lib/CodeGen/CGExprConstant.cpp
@@ -650,6 +650,8 @@
   }
 
   if (NonzeroLength == 0) {
+if (DestType == nullptr)
+  return nullptr;
 return llvm::ConstantAggregateZero::get(
 CGM.getTypes().ConvertType(QualType(DestType, 0)));
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits