[clang] [clang] ASTContex: fix getCommonSugaredType for array types (PR #132559)

[Matheus Izvekov via cfe-commits]

https://github.com/mizvekov updated https://github.com/llvm/llvm-project/pull/132559

>From 27cf231f8497cc483c44d5e5ada05af9ae48e6ab Mon Sep 17 00:00:00 2001
From: Matheus Izvekov <mizvekov at gmail.com>
Date: Sat, 22 Mar 2025 14:39:10 -0300
Subject: [PATCH] [clang] ASTContex: fix getCommonSugaredType for array types

This corrects the behaviour for getCommonSugaredType with regards
to array top level qualifiers: remove differing top level qualifiers, as
they must be redundant with element qualifiers.

Fixes https://github.com/llvm/llvm-project/issues/97005
---
 clang/docs/ReleaseNotes.rst               |  3 ++
 clang/lib/AST/ASTContext.cpp              | 22 ++++++++++++-
 clang/test/SemaCXX/sugar-common-types.cpp | 40 +++++++++++++++++++++++
 3 files changed, 64 insertions(+), 1 deletion(-)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 8182bccdd2da8..89fd193fde739 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -340,6 +340,9 @@ Bug Fixes to C++ Support
   by template argument deduction.
 - Clang is now better at instantiating the function definition after its use inside
   of a constexpr lambda. (#GH125747)
+- Clang no longer crashes when trying to unify the types of arrays with
+  certain differences in qualifiers, per ternary operator or template argument
+  deduction. (#GH97005)
 - The initialization kind of elements of structured bindings
   direct-list-initialized from an array is corrected to direct-initialization.
 - Clang no longer crashes when a coroutine is declared ``[[noreturn]]``. (#GH127327)
diff --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp
index de868ac821745..f068ecbd0408d 100644
--- a/clang/lib/AST/ASTContext.cpp
+++ b/clang/lib/AST/ASTContext.cpp
@@ -13650,7 +13650,11 @@ static QualType getCommonArrayElementType(ASTContext &Ctx, const T *X,
   QualType EX = X->getElementType(), EY = Y->getElementType();
   QualType R = Ctx.getCommonSugaredType(EX, EY,
                                         /*Unqualified=*/true);
+  // Qualifiers common to both element types.
   Qualifiers RQ = R.getQualifiers();
+  // For each side, move to the top level any qualifiers which are not common to
+  // both element types. The caller must assume top level qualifiers might
+  // be different, even if they are the same type, and can be treated as sugar.
   QX += EX.getQualifiers() - RQ;
   QY += EY.getQualifiers() - RQ;
   return R;
@@ -14371,6 +14375,22 @@ QualType ASTContext::getCommonSugaredType(QualType X, QualType Y,
   // necessarily canonical types, as they may still have sugared properties.
   // QX and QY will store the sum of all qualifiers in Xs and Ys respectively.
   auto Xs = ::unwrapSugar(SX, QX), Ys = ::unwrapSugar(SY, QY);
+
+  // If this is an ArrayType, the element qualifiers are interchangeable with
+  // the top level qualifiers.
+  // * In case the canonical nodes are the same, the elements types are already
+  // the same.
+  // * Otherwise, the element types will be made the same, and any different
+  // element qualifiers will be pumped up to the top level qualifiers, per
+  // 'getCommonArrayElementType'.
+  // In both cases, this means there may be top level qualifiers which differ
+  // between X and Y. If so, these differing qualifiers are redundant with the
+  // element qualifiers, and can be removed without changing the canonical type.
+  // The desired behaviour is the same as for the 'Unqualified' case here:
+  // treat the redundant qualifiers as sugar, remove the ones which are not
+  // common to both sides.
+  bool KeepCommonQualifiers = Unqualified || isa<ArrayType>(SX.Ty);
+
   if (SX.Ty != SY.Ty) {
     // The canonical nodes differ. Build a common canonical node out of the two,
     // unifying their sugar. This may recurse back here.
@@ -14386,7 +14406,7 @@ QualType ASTContext::getCommonSugaredType(QualType X, QualType Y,
       SY = Ys.pop_back_val();
     }
   }
-  if (Unqualified)
+  if (KeepCommonQualifiers)
     QX = Qualifiers::removeCommonQualifiers(QX, QY);
   else
     assert(QX == QY);
diff --git a/clang/test/SemaCXX/sugar-common-types.cpp b/clang/test/SemaCXX/sugar-common-types.cpp
index 3c4de0e2abd5e..a21032517b2ba 100644
--- a/clang/test/SemaCXX/sugar-common-types.cpp
+++ b/clang/test/SemaCXX/sugar-common-types.cpp
@@ -146,3 +146,43 @@ namespace GH67603 {
   }
   template void h<int>();
 } // namespace GH67603
+
+namespace arrays {
+  namespace same_canonical {
+    using ConstB1I = const B1[];
+    using ConstB1C = const B1[1];
+    const ConstB1I a = {0};
+    const ConstB1C b = {0};
+    N ta = a;
+    // expected-error at -1 {{lvalue of type 'const B1[1]' (aka 'const int[1]')}}
+    N tb = b;
+    // expected-error at -1 {{lvalue of type 'const ConstB1C' (aka 'const const int[1]')}}
+    N tc = 0 ? a : b;
+    // expected-error at -1 {{lvalue of type 'const B1[1]' (aka 'const int[1]')}}
+  } // namespace same_canonical
+  namespace same_element {
+    using ConstB1 = const B1;
+    using ConstB1I = ConstB1[];
+    using ConstB1C = ConstB1[1];
+    const ConstB1I a = {0};
+    const ConstB1C b = {0};
+    N ta = a;
+    // expected-error at -1 {{lvalue of type 'const ConstB1[1]' (aka 'const int[1]')}}
+    N tb = b;
+    // expected-error at -1 {{lvalue of type 'const ConstB1C' (aka 'const const int[1]')}}
+    N tc = 0 ? a : b;
+    // expected-error at -1 {{lvalue of type 'ConstB1[1]' (aka 'const int[1]')}}
+  } // namespace same_element
+  namespace balanced_qualifiers {
+    using ConstX1C = const volatile X1[1];
+    using Y1C = volatile Y1[1];
+    extern volatile ConstX1C a;
+    extern const volatile Y1C b;
+    N ta = a;
+    // expected-error at -1 {{lvalue of type 'volatile ConstX1C' (aka 'volatile const volatile int[1]')}}
+    N tb = b;
+    // expected-error at -1 {{lvalue of type 'const volatile Y1C' (aka 'const volatile volatile int[1]')}}
+    N tc = 0 ? a : b;
+    // expected-error at -1 {{lvalue of type 'const volatile volatile B1[1]' (aka 'const volatile volatile int[1]')}}
+  } // namespace balanced_qualifiers
+} // namespace arrays