Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions clang/docs/ReleaseNotes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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 (this could happen during template argument
deduction or when building a ternary operator). (#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)
Expand Down
22 changes: 21 additions & 1 deletion clang/lib/AST/ASTContext.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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 moved 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.
Expand All @@ -14386,7 +14406,7 @@ QualType ASTContext::getCommonSugaredType(QualType X, QualType Y,
SY = Ys.pop_back_val();
}
}
if (Unqualified)
if (KeepCommonQualifiers)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I apologize if I am being dense here but having if KeepCommonQualifiers and then going on to calling removeCommonQualifiers feels confusing.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, removeCommonQualifiers takes two parameters by reference, and removes their common qualifiers modifying them in place...
BUT, then it returns the common qualifiers, which is what we are interested, the rest is discarded.

QX = Qualifiers::removeCommonQualifiers(QX, QY);
else
assert(QX == QY);
Expand Down
40 changes: 40 additions & 0 deletions clang/test/SemaCXX/sugar-common-types.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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@-1 {{lvalue of type 'const B1[1]' (aka 'const int[1]')}}
N tb = b;
// expected-error@-1 {{lvalue of type 'const ConstB1C' (aka 'const const int[1]')}}
N tc = 0 ? a : b;
// expected-error@-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@-1 {{lvalue of type 'const ConstB1[1]' (aka 'const int[1]')}}
N tb = b;
// expected-error@-1 {{lvalue of type 'const ConstB1C' (aka 'const const int[1]')}}
N tc = 0 ? a : b;
// expected-error@-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@-1 {{lvalue of type 'volatile ConstX1C' (aka 'volatile const volatile int[1]')}}
N tb = b;
// expected-error@-1 {{lvalue of type 'const volatile Y1C' (aka 'const volatile volatile int[1]')}}
N tc = 0 ? a : b;
// expected-error@-1 {{lvalue of type 'const volatile volatile B1[1]' (aka 'const volatile volatile int[1]')}}
} // namespace balanced_qualifiers
} // namespace arrays