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

[via cfe-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-clang

Author: Matheus Izvekov (mizvekov)

<details>
<summary>Changes</summary>

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

---
Full diff: https://github.com/llvm/llvm-project/pull/132559.diff


3 Files Affected:

- (modified) clang/docs/ReleaseNotes.rst (+3) 
- (modified) clang/lib/AST/ASTContext.cpp (+17-1) 
- (modified) clang/test/SemaCXX/sugar-common-types.cpp (+40) 


``````````diff
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..365565590303f 100644
--- a/clang/lib/AST/ASTContext.cpp
+++ b/clang/lib/AST/ASTContext.cpp
@@ -14371,6 +14371,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 +14402,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

``````````

</details>


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