[clang] 343bed8 - Canonically identical types are allowed in compound expressions in C

[Aaron Ballman via cfe-commits]

Author: Aaron Ballman
Date: 2023-10-14T11:03:35-04:00
New Revision: 343bed8d3a9b632594a3f786bbb189613975d51e

URL: https://github.com/llvm/llvm-project/commit/343bed8d3a9b632594a3f786bbb189613975d51e
DIFF: https://github.com/llvm/llvm-project/commit/343bed8d3a9b632594a3f786bbb189613975d51e.diff

LOG: Canonically identical types are allowed in compound expressions in C

We did not have a catch-all for when the two operand types are
identical after canonicalization. Instead, we handled that on a case by
case basis. Thus, we would diagnose code like:
```
mat4 test(int a) {
  typedef float mat4 __attribute((matrix_type(4, 4)));
  mat4 transform;
  return (a > 0) ? transform : transform;
}
```
This simplifies the logic and will be more forwards
compatible with other extended datatypes.

Fixes https://github.com/llvm/llvm-project/issues/69008

Added: 
    

Modified: 
    clang/docs/ReleaseNotes.rst
    clang/lib/Sema/SemaExpr.cpp
    clang/test/Sema/conditional.c

Removed: 
    


################################################################################
diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index ade3c33b3b9444c..be7c8bf247f7af5 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -386,10 +386,13 @@ Bug Fixes in This Version
   cannot be used with ``Release`` mode builds. (`#68237 <https://github.com/llvm/llvm-project/issues/68237>`_).
 - Fix crash in evaluating ``constexpr`` value for invalid template function.
   Fixes (`#68542 <https://github.com/llvm/llvm-project/issues/68542>`_)
-
 - Fixed an issue when a shift count larger than ``__INT64_MAX__``, in a right
   shift operation, could result in missing warnings about
   ``shift count >= width of type`` or internal compiler error.
+- Fixed an issue with computing the common type for the LHS and RHS of a `?:`
+  operator in C. No longer issuing a confusing diagnostic along the lines of
+  "incompatible operand types ('foo' and 'foo')" with extensions such as matrix
+  types. Fixes (`#69008 <https://github.com/llvm/llvm-project/issues/69008>`_)
 
 Bug Fixes to Compiler Builtins
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

diff  --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index d78f923b2cb2cb3..aa30a3a03887558 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -9186,7 +9186,7 @@ QualType Sema::CheckConditionalOperands(ExprResult &Cond, ExprResult &LHS,
   if (checkCondition(*this, Cond.get(), QuestionLoc))
     return QualType();
 
-  // Now check the two expressions.
+  // Handle vectors.
   if (LHS.get()->getType()->isVectorType() ||
       RHS.get()->getType()->isVectorType())
     return CheckVectorOperands(LHS, RHS, QuestionLoc, /*isCompAssign*/ false,
@@ -9244,11 +9244,6 @@ QualType Sema::CheckConditionalOperands(ExprResult &Cond, ExprResult &LHS,
     return ResTy;
   }
 
-  // And if they're both bfloat (which isn't arithmetic), that's fine too.
-  if (LHSTy->isBFloat16Type() && RHSTy->isBFloat16Type()) {
-    return Context.getCommonSugaredType(LHSTy, RHSTy);
-  }
-
   // If both operands are the same structure or union type, the result is that
   // type.
   if (const RecordType *LHSRT = LHSTy->getAs<RecordType>()) {    // C99 6.5.15p3
@@ -9320,17 +9315,17 @@ QualType Sema::CheckConditionalOperands(ExprResult &Cond, ExprResult &LHS,
       /*IsIntFirstExpr=*/false))
     return LHSTy;
 
-  // Allow ?: operations in which both operands have the same
-  // built-in sizeless type.
-  if (LHSTy->isSizelessBuiltinType() && Context.hasSameType(LHSTy, RHSTy))
-    return Context.getCommonSugaredType(LHSTy, RHSTy);
-
   // Emit a better diagnostic if one of the expressions is a null pointer
   // constant and the other is not a pointer type. In this case, the user most
   // likely forgot to take the address of the other expression.
   if (DiagnoseConditionalForNull(LHS.get(), RHS.get(), QuestionLoc))
     return QualType();
 
+  // Finally, if the LHS and RHS types are canonically the same type, we can
+  // use the common sugared type.
+  if (Context.hasSameType(LHSTy, RHSTy))
+    return Context.getCommonSugaredType(LHSTy, RHSTy);
+
   // Otherwise, the operands are not compatible.
   Diag(QuestionLoc, diag::err_typecheck_cond_incompatible_operands)
     << LHSTy << RHSTy << LHS.get()->getSourceRange()

diff  --git a/clang/test/Sema/conditional.c b/clang/test/Sema/conditional.c
index 666ac5416322d5d..cebdb7b4043a392 100644
--- a/clang/test/Sema/conditional.c
+++ b/clang/test/Sema/conditional.c
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 %s -fsyntax-only -verify
+// RUN: %clang_cc1 %s -fsyntax-only -fenable-matrix -verify
 
 const char* test1 = 1 ? "i" : 1 == 1 ? "v" : "r";
 
@@ -19,3 +19,15 @@ void pr39809(void) {
   _Generic(0 ? (int volatile*)0 : (void const*)1, void volatile const*: (void)0);
   _Generic(0 ? (int volatile*)0 : (void const*)0, void volatile const*: (void)0);
 }
+
+// Ensure we compute the correct common type for extension types as well.
+void GH69008(void) {
+  typedef float mat4 __attribute((matrix_type(4, 4)));
+  typedef float mat5 __attribute((matrix_type(5, 5)));
+
+  mat4 transform;
+  (void)(1 ? transform : transform); // ok
+
+  mat5 other_transform;
+  (void)(1 ? other_transform : transform); // expected-error {{incompatible operand types ('mat5' (aka 'float __attribute__((matrix_type(5, 5)))') and 'mat4' (aka 'float __attribute__((matrix_type(4, 4)))'))}}
+}