On Monday 02 Apr 2012 16:13:11 Eli Friedman wrote:
> On Mon, Apr 2, 2012 at 4:46 AM, Tim Northover <[email protected]> wrote:
> > Hi,
> > 
> > When doing something like this:
> > 
> > const int *CI;
> > int *I;
> > *(test ? I : CI) = 0
> > 
> > clang currently doesn't put the combined qualifiers on the output type,
> > which means that the assignment is allowed. I believe this patch
> > implements correct C99 semantics (and sane behaviour for non-CVR type
> > qualifiers).
> > 
> > Could someone review it please?
> 
> It would be nice to have tests for both "x ? incomplete : complete"
> and "z ? complete : incomplete" for the tests involving composite
> types.

Thanks Eli. I've attached an updated patch (also adds a two-way test for the 
const bit, but no other changes).

Could you commit it if it's OK? I don't have access yet.

Cheers.

Tim.
>From 597caad42e8a2eaa53d55ecd4303f52342e25216 Mon Sep 17 00:00:00 2001
From: Tim Northover <[email protected]>
Date: Sat, 31 Mar 2012 19:54:56 +0100
Subject: [PATCH] Implement merging of types in conditional expressions

---
 lib/Sema/SemaExpr.cpp        |   38 +++++++++++++++++++++++++++-----------
 test/Sema/conditional-expr.c |   17 +++++++++++++++++
 2 files changed, 44 insertions(+), 11 deletions(-)

diff --git a/lib/Sema/SemaExpr.cpp b/lib/Sema/SemaExpr.cpp
index 97cb647..130406c 100644
--- a/lib/Sema/SemaExpr.cpp
+++ b/lib/Sema/SemaExpr.cpp
@@ -4567,8 +4567,28 @@ static QualType checkConditionalPointerCompatibility(Sema &S, ExprResult &LHS,
     rhptee = RHSTy->castAs<PointerType>()->getPointeeType();
   }
 
-  if (!S.Context.typesAreCompatible(lhptee.getUnqualifiedType(),
-                                    rhptee.getUnqualifiedType())) {
+  // C99 6.5.15p6: If both operands are pointers to compatible types or to
+  // differently qualified versions of compatible types, the result type is
+  // a pointer to an appropriately qualified version of the composite
+  // type.
+
+  // Only CVR-qualifiers exist in the standard, and the differently-qualified
+  // clause doesn't make sense for our extensions. E.g. address space 2 should
+  // be incompatible with address space 3: they may live on different devices or
+  // anything.
+  Qualifiers lhQual = lhptee.getQualifiers();
+  Qualifiers rhQual = rhptee.getQualifiers();
+
+  unsigned MergedCVRQual = lhQual.getCVRQualifiers() | rhQual.getCVRQualifiers();
+  lhQual.removeCVRQualifiers();
+  rhQual.removeCVRQualifiers();
+
+  lhptee = S.Context.getQualifiedType(lhptee.getUnqualifiedType(), lhQual);
+  rhptee = S.Context.getQualifiedType(rhptee.getUnqualifiedType(), rhQual);
+
+  QualType CompositeTy = S.Context.mergeTypes(lhptee, rhptee);
+
+  if (CompositeTy.isNull()) {
     S.Diag(Loc, diag::warn_typecheck_cond_incompatible_pointers)
       << LHSTy << RHSTy << LHS.get()->getSourceRange()
       << RHS.get()->getSourceRange();
@@ -4582,16 +4602,12 @@ static QualType checkConditionalPointerCompatibility(Sema &S, ExprResult &LHS,
   }
 
   // The pointer types are compatible.
-  // C99 6.5.15p6: If both operands are pointers to compatible types *or* to
-  // differently qualified versions of compatible types, the result type is
-  // a pointer to an appropriately qualified version of the *composite*
-  // type.
-  // FIXME: Need to calculate the composite type.
-  // FIXME: Need to add qualifiers
+  QualType ResultTy = CompositeTy.withCVRQualifiers(MergedCVRQual);
+  ResultTy = S.Context.getPointerType(ResultTy);
 
-  LHS = S.ImpCastExprToType(LHS.take(), LHSTy, CK_BitCast);
-  RHS = S.ImpCastExprToType(RHS.take(), LHSTy, CK_BitCast);
-  return LHSTy;
+  LHS = S.ImpCastExprToType(LHS.take(), ResultTy, CK_BitCast);
+  RHS = S.ImpCastExprToType(RHS.take(), ResultTy, CK_BitCast);
+  return ResultTy;
 }
 
 /// \brief Return the resulting type when the operands are both block pointers.
diff --git a/test/Sema/conditional-expr.c b/test/Sema/conditional-expr.c
index 436ecdb..184ac4a 100644
--- a/test/Sema/conditional-expr.c
+++ b/test/Sema/conditional-expr.c
@@ -60,6 +60,23 @@ void foo() {
   test0 = test0 ? EVal : test1; // expected-warning {{operand of ? changes signedness: 'int' to 'unsigned long'}}
   test0 = test0 ? test1 : EVal; // expected-warning {{operand of ? changes signedness: 'int' to 'unsigned long'}}
 
+  const int *const_int;
+  int *nonconst_int;
+  *(test0 ? const_int : nonconst_int) = 42; // expected-error {{read-only variable is not assignable}}
+  *(test0 ? nonconst_int : const_int) = 42; // expected-error {{read-only variable is not assignable}}
+
+  // The composite type here should be "int (*)[12]", fine for the sizeof
+  int (*incomplete)[];
+  int (*complete)[12];
+  sizeof(*(test0 ? incomplete : complete)); // expected-warning {{expression result unused}}
+  sizeof(*(test0 ? complete : incomplete)); // expected-warning {{expression result unused}}
+
+  int __attribute__((address_space(2))) *adr2;
+  int __attribute__((address_space(3))) *adr3;
+  test0 ? adr2 : adr3; // expected-warning {{pointer type mismatch}} expected-warning {{expression result unused}}
+
+  // Make sure address-space mask ends up in the result type
+  (test0 ? (test0 ? adr2 : adr2) : nonconst_int); // expected-warning {{pointer type mismatch}} expected-warning {{expression result unused}}
 }
 
 int Postgresql() {
-- 
1.7.9.5
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Reply via email to