[PATCH] Gracefully (and correctly) handle init of multiple union members
Matthew Curtis
mcurtis at codeaurora.org
Tue Sep 24 14:06:21 PDT 2013
New patch incorporating Eli's feedback.
Thanks Eli!
Matthew Curtis
On 9/24/2013 2:07 PM, Eli Friedman wrote:
> On Tue, Sep 24, 2013 at 8:04 AM, Matthew Curtis
> <mcurtis at codeaurora.org <mailto:mcurtis at codeaurora.org>> wrote:
>
> We now emit warnings when doing so and code generation is consistent
> with GCC. Note that the C99 spec is unclear as to the precise
> behavior.
>
> See also ...
> Bug:
> http://llvm.org/bugs/show_bug.cgi?id=16644 and
>
> cfe-dev discussion:
> http://lists.cs.uiuc.edu/pipermail/cfe-dev/2013-September/031918.html
>
>
>
> + FieldDecl *CFD = ArrayFillerOrUnionFieldInit.dyn_cast<FieldDecl *>();
> + assert((FD == 0 || CFD == 0 || CFD == FD)
> + && "Only one field of a union may be initiazed at a time!");
>
> This will cause an unused variable warning in release builds. Also,
> spelling.
>
> diff --git a/test/Sema/designated-initializers.c
> b/test/Sema/designated-initializers.c
> index 36fa559..be365a0 100644
> --- a/test/Sema/designated-initializers.c
> +++ b/test/Sema/designated-initializers.c
> @@ -1,5 +1,8 @@
> // RUN: %clang_cc1 -fsyntax-only -verify -triple
> x86_64-unknown-unknown %s
> +// RUN: %clang_cc1 -triple x86_64-unknown-unknown %s -DCHECK_CODEGEN=1 \
> +// RUN: -S -emit-llvm -o - 2>&1 | FileCheck %s
>
> Please put a separate code generation test into test/CodeGen. It's
> okay if you duplicate the test code.
>
> The approach looks fine.
>
> -Eli
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130924/1ee5af89/attachment.html>
-------------- next part --------------
>From d3ad9345e0465b44614e9c9625a34dd2ab9bb491 Mon Sep 17 00:00:00 2001
From: Matthew Curtis <mcurtis at codeaurora.org>
Date: Fri, 13 Sep 2013 11:09:37 -0500
Subject: [PATCH] Gracefully (and correctly) handle init of multiple union
members
We now emit warnings when doing so and code generation is consistent
with GCC. Note that the C99 spec is unclear as to the precise
behavior.
See also ...
Bug:
http://llvm.org/bugs/show_bug.cgi?id=16644 and
cfe-dev discussion:
http://lists.cs.uiuc.edu/pipermail/cfe-dev/2013-September/031918.html
---
include/clang/AST/Expr.h | 4 ++
lib/Sema/SemaInit.cpp | 23 ++++++++++++++-
test/CodeGen/designated-initializers.c | 49 ++++++++++++++++++++++++++++++++
test/Sema/designated-initializers.c | 43 +++++++++++++++++++++++++++-
4 files changed, 117 insertions(+), 2 deletions(-)
diff --git a/include/clang/AST/Expr.h b/include/clang/AST/Expr.h
index 8a884c0..a53b88d 100644
--- a/include/clang/AST/Expr.h
+++ b/include/clang/AST/Expr.h
@@ -3824,6 +3824,10 @@ public:
return const_cast<InitListExpr *>(this)->getInitializedFieldInUnion();
}
void setInitializedFieldInUnion(FieldDecl *FD) {
+ assert((FD == 0
+ || getInitializedFieldInUnion() == 0
+ || getInitializedFieldInUnion() == FD)
+ && "Only one field of a union may be initialized at a time!");
ArrayFillerOrUnionFieldInit = FD;
}
diff --git a/lib/Sema/SemaInit.cpp b/lib/Sema/SemaInit.cpp
index afd9393..e407253 100644
--- a/lib/Sema/SemaInit.cpp
+++ b/lib/Sema/SemaInit.cpp
@@ -1868,8 +1868,29 @@ InitListChecker::CheckDesignatedInitializer(const InitializedEntity &Entity,
// the initializer list.
if (RT->getDecl()->isUnion()) {
FieldIndex = 0;
- if (!VerifyOnly)
+ if (!VerifyOnly) {
+ FieldDecl *CurrentField = StructuredList->getInitializedFieldInUnion();
+ if (CurrentField && CurrentField != *Field) {
+ assert(StructuredList->getNumInits() == 1
+ && "A union should never have more than one initializer!");
+
+ // we're about to throw away an initializer, emit warning
+ SemaRef.Diag(D->getFieldLoc(),
+ diag::warn_initializer_overrides)
+ << D->getSourceRange();
+ Expr *ExistingInit = StructuredList->getInit(0);
+ SemaRef.Diag(ExistingInit->getLocStart(),
+ diag::note_previous_initializer)
+ << /*FIXME:has side effects=*/0
+ << ExistingInit->getSourceRange();
+
+ // remove existing initializer
+ StructuredList->resizeInits(SemaRef.Context, 0);
+ StructuredList->setInitializedFieldInUnion(0);
+ }
+
StructuredList->setInitializedFieldInUnion(*Field);
+ }
}
// Make sure we can use this declaration.
diff --git a/test/CodeGen/designated-initializers.c b/test/CodeGen/designated-initializers.c
index b418986..06b8f13 100644
--- a/test/CodeGen/designated-initializers.c
+++ b/test/CodeGen/designated-initializers.c
@@ -81,6 +81,55 @@ struct overwrite_string_struct5 {
// CHECK: [6 x i8] c"foo\00y\00", i32 1
+// CHECK: @u1 = {{.*}} { i32 65535 }
+union u_FFFF { char c; long l; } u1 = { .l = 0xFFFF };
+
+
+/// PR16644
+typedef union u_16644 {
+ struct s_16644 {
+ int zero;
+ int one;
+ int two;
+ int three;
+ } a;
+ int b[4];
+} union_16644_t;
+
+// CHECK: @union_16644_instance_0 = {{.*}} { i32 0, i32 0, i32 0, i32 3 } }
+union_16644_t union_16644_instance_0 =
+{
+ .b[0] = 0,
+ .a.one = 1,
+ .b[2] = 2,
+ .a.three = 3,
+};
+
+// CHECK: @union_16644_instance_1 = {{.*}} [i32 10, i32 0, i32 0, i32 0]
+union_16644_t union_16644_instance_1 =
+{
+ .a.three = 13,
+ .b[2] = 12,
+ .a.one = 11,
+ .b[0] = 10,
+};
+
+// CHECK: @union_16644_instance_2 = {{.*}} [i32 0, i32 20, i32 0, i32 0]
+union_16644_t union_16644_instance_2 =
+{
+ .a.one = 21,
+ .b[1] = 20,
+};
+
+// CHECK: @union_16644_instance_3 = {{.*}} { i32 0, i32 31, i32 0, i32 0 }
+union_16644_t union_16644_instance_3 =
+{
+ .b[1] = 30,
+ .a = {
+ .one = 31
+ }
+};
+
void test1(int argc, char **argv)
{
diff --git a/test/Sema/designated-initializers.c b/test/Sema/designated-initializers.c
index 36fa559..29d86b4 100644
--- a/test/Sema/designated-initializers.c
+++ b/test/Sema/designated-initializers.c
@@ -137,7 +137,6 @@ void test() {
};
}
-// FIXME: How do we test that this initializes the long properly?
union { char c; long l; } u1 = { .l = 0xFFFF };
extern float global_float;
@@ -223,6 +222,48 @@ struct Enigma enigma = {
};
+/// PR16644
+typedef union {
+ struct {
+ int zero;
+ int one;
+ int two;
+ int three;
+ } a;
+ int b[4];
+} union_16644_t;
+
+union_16644_t union_16644_instance_0 =
+{
+ .b[0] = 0, // expected-note{{previous}}
+ .a.one = 1, // expected-warning{{overrides}} expected-note{{previous}}
+ .b[2] = 2, // expected-warning{{overrides}} expected-note{{previous}}
+ .a.three = 3, // expected-warning{{overrides}}
+};
+
+union_16644_t union_16644_instance_1 =
+{
+ .a.three = 13, // expected-note{{previous}}
+ .b[2] = 12, // expected-warning{{overrides}} expected-note{{previous}}
+ .a.one = 11, // expected-warning{{overrides}} expected-note{{previous}}
+ .b[0] = 10, // expected-warning{{overrides}}
+};
+
+union_16644_t union_16644_instance_2 =
+{
+ .a.one = 21, // expected-note{{previous}}
+ .b[1] = 20, // expected-warning{{overrides}}
+};
+
+union_16644_t union_16644_instance_3 =
+{
+ .b[1] = 30, // expected-note{{previous}}
+ .a = { // expected-warning{{overrides}}
+ .one = 31
+ }
+};
+
+
/// PR4073
/// Should use evaluate to fold aggressively and emit a warning if not an ice.
extern int crazy_x;
--
1.7.8.3
More information about the cfe-commits
mailing list