Skip to content

Commit

Permalink
Canonically identical types are allowed in compound expressions in C
Browse files Browse the repository at this point in the history
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 #69008
  • Loading branch information
AaronBallman committed Oct 14, 2023
1 parent 5e1c2bf commit 343bed8
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 13 deletions.
5 changes: 4 additions & 1 deletion clang/docs/ReleaseNotes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Expand Down
17 changes: 6 additions & 11 deletions clang/lib/Sema/SemaExpr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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()
Expand Down
14 changes: 13 additions & 1 deletion clang/test/Sema/conditional.c
Original file line number Diff line number Diff line change
@@ -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";

Expand All @@ -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)))'))}}
}

0 comments on commit 343bed8

Please sign in to comment.