[clang] [NFC][clang] change remaining context-dependent type nodes to ContextualFoldingSet (PR #67751)

[Matheus Izvekov via cfe-commits]

https://github.com/mizvekov created 
https://github.com/llvm/llvm-project/pull/67751

Stacked, consider only last commit.

>From 7009ebf7f2016816b5cbb694e0611cd2a86d7cf1 Mon Sep 17 00:00:00 2001
From: Matheus Izvekov 
Date: Thu, 28 Sep 2023 22:38:59 +0200
Subject: [PATCH 1/2] [clang] implement common sugared type of inst-dependent
 DecltypeType

While a DecltypeType node itself is not uniqued, an
instantiation dependent DecltypeType will have a
DependentDecltypeType as an underlying type, which is uniqued.

In that case, there can be non-identical non-sugar DecltypeTypes nodes
which nonetheless represent the same type.

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

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 1d74c492845a6c3..09302040a3510b6 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -266,6 +266,9 @@ Bug Fixes in This Version
   (`#64836 `_)
 - Clang now allows an ``_Atomic`` qualified integer in a switch statement. 
Fixes
   (`#65557 `_)
+- Fixes crash when trying to obtain the common sugared type of
+  `decltype(instantiation-dependent-expr)`.
+  Fixes (`#67603 `_)
 
 Bug Fixes to Compiler Builtins
 ^^
diff --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp
index 57aaa05b1d81ddb..5ce0b54166e0255 100644
--- a/clang/lib/AST/ASTContext.cpp
+++ b/clang/lib/AST/ASTContext.cpp
@@ -12705,7 +12705,6 @@ static QualType getCommonNonSugarTypeNode(ASTContext 
&Ctx, const Type *X,
 
 #define SUGAR_FREE_TYPE(Class) UNEXPECTED_TYPE(Class, "sugar-free")
 SUGAR_FREE_TYPE(Builtin)
-SUGAR_FREE_TYPE(Decltype)
 SUGAR_FREE_TYPE(DeducedTemplateSpecialization)
 SUGAR_FREE_TYPE(DependentBitInt)
 SUGAR_FREE_TYPE(Enum)
@@ -12935,6 +12934,14 @@ static QualType getCommonNonSugarTypeNode(ASTContext 
&Ctx, const Type *X,
TY->getTemplateName()),
 As, X->getCanonicalTypeInternal());
   }
+  case Type::Decltype: {
+const auto *DX = cast(X), *DY = cast(Y);
+assert(DX->isDependentType());
+assert(DY->isDependentType());
+assert(Ctx.hasSameExpr(DX->getUnderlyingExpr(), DY->getUnderlyingExpr()));
+// As Decltype is not uniqued, building a common type would be wasteful.
+return QualType(DX, 0);
+  }
   case Type::DependentName: {
 const auto *NX = cast(X),
*NY = cast(Y);
diff --git a/clang/test/SemaCXX/sugar-common-types.cpp 
b/clang/test/SemaCXX/sugar-common-types.cpp
index 4a8ff2addb66359..e1c7578a66b9cad 100644
--- a/clang/test/SemaCXX/sugar-common-types.cpp
+++ b/clang/test/SemaCXX/sugar-common-types.cpp
@@ -142,3 +142,14 @@ namespace PR61419 {
   extern const pair p;
   id t = false ? p.first : p.second;
 } // namespace PR61419
+
+namespace GH67603 {
+  template  using A = long;
+  template  void h() {
+using C = B;
+using D = B;
+N t = 0 ? A() : A();
+// expected-error@-1 {{rvalue of type 'A' (aka 'long')}}
+  }
+  template void h();
+} // namespace GH67603

>From b6f194c89bafad17e1dcc76b3a87d2330aac1c10 Mon Sep 17 00:00:00 2001
From: Matheus Izvekov 
Date: Fri, 29 Sep 2023 00:49:16 +0200
Subject: [PATCH 2/2] [NFC][clang] change remaining context-dependent type
 nodes to ContextualFoldingSet

---
 clang/include/clang/AST/ASTContext.h | 24 ++
 clang/include/clang/AST/Type.h   | 62 ++---
 clang/lib/AST/ASTContext.cpp | 68 
 clang/lib/AST/Type.cpp   | 47 +--
 4 files changed, 90 insertions(+), 111 deletions(-)

diff --git a/clang/include/clang/AST/ASTContext.h 
b/clang/include/clang/AST/ASTContext.h
index 4ee32c76a95d8e3..8ad0514ee2ce227 100644
--- a/clang/include/clang/AST/ASTContext.h
+++ b/clang/include/clang/AST/ASTContext.h
@@ -195,20 +195,25 @@ class ASTContext : public RefCountedBase {
   ConstantArrayTypes;
   mutable llvm::FoldingSet IncompleteArrayTypes;
   mutable std::vector VariableArrayTypes;
-  mutable llvm::FoldingSet DependentSizedArrayTypes;
-  mutable llvm::FoldingSet
-DependentSizedExtVectorTypes;
-  mutable llvm::FoldingSet
+  mutable llvm::ContextualFoldingSet
+  DependentSizedArrayTypes;
+  mutable llvm::ContextualFoldingSet
+  DependentSizedExtVectorTypes;
+  mutable llvm::ContextualFoldingSet
   DependentAddressSpaceTypes;
   mutable llvm::FoldingSet VectorTypes;
-  mutable llvm::FoldingSet DependentVectorTypes;
+  mutable llvm::ContextualFoldingSet
+  DependentVectorTypes;
   mutable llvm::FoldingSet MatrixTypes;
-  mutable llvm::FoldingSet DependentSizedMatrixTypes;
+  mutable l

[clang] [NFC][clang] change remaining context-dependent type nodes to ContextualFoldingSet (PR #67751)

[via cfe-commits]

llvmbot wrote:




@llvm/pr-subscribers-clang


Changes

Stacked, consider only last commit.

---

Patch is 28.71 KiB, truncated to 20.00 KiB below, full version: 
https://github.com/llvm/llvm-project/pull/67751.diff


6 Files Affected:

- (modified) clang/docs/ReleaseNotes.rst (+3) 
- (modified) clang/include/clang/AST/ASTContext.h (+15-9) 
- (modified) clang/include/clang/AST/Type.h (+23-39) 
- (modified) clang/lib/AST/ASTContext.cpp (+38-39) 
- (modified) clang/lib/AST/Type.cpp (+22-25) 
- (modified) clang/test/SemaCXX/sugar-common-types.cpp (+11) 


``diff
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 1d74c492845a6c3..09302040a3510b6 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -266,6 +266,9 @@ Bug Fixes in This Version
   (`#64836 `_)
 - Clang now allows an ``_Atomic`` qualified integer in a switch statement. 
Fixes
   (`#65557 `_)
+- Fixes crash when trying to obtain the common sugared type of
+  `decltype(instantiation-dependent-expr)`.
+  Fixes (`#67603 `_)
 
 Bug Fixes to Compiler Builtins
 ^^
diff --git a/clang/include/clang/AST/ASTContext.h 
b/clang/include/clang/AST/ASTContext.h
index 4ee32c76a95d8e3..8ad0514ee2ce227 100644
--- a/clang/include/clang/AST/ASTContext.h
+++ b/clang/include/clang/AST/ASTContext.h
@@ -195,20 +195,25 @@ class ASTContext : public RefCountedBase {
   ConstantArrayTypes;
   mutable llvm::FoldingSet IncompleteArrayTypes;
   mutable std::vector VariableArrayTypes;
-  mutable llvm::FoldingSet DependentSizedArrayTypes;
-  mutable llvm::FoldingSet
-DependentSizedExtVectorTypes;
-  mutable llvm::FoldingSet
+  mutable llvm::ContextualFoldingSet
+  DependentSizedArrayTypes;
+  mutable llvm::ContextualFoldingSet
+  DependentSizedExtVectorTypes;
+  mutable llvm::ContextualFoldingSet
   DependentAddressSpaceTypes;
   mutable llvm::FoldingSet VectorTypes;
-  mutable llvm::FoldingSet DependentVectorTypes;
+  mutable llvm::ContextualFoldingSet
+  DependentVectorTypes;
   mutable llvm::FoldingSet MatrixTypes;
-  mutable llvm::FoldingSet DependentSizedMatrixTypes;
+  mutable llvm::ContextualFoldingSet
+  DependentSizedMatrixTypes;
   mutable llvm::FoldingSet FunctionNoProtoTypes;
   mutable llvm::ContextualFoldingSet
 FunctionProtoTypes;
-  mutable llvm::FoldingSet DependentTypeOfExprTypes;
-  mutable llvm::FoldingSet DependentDecltypeTypes;
+  mutable llvm::ContextualFoldingSet
+  DependentTypeOfExprTypes;
+  mutable llvm::ContextualFoldingSet
+  DependentDecltypeTypes;
   mutable llvm::FoldingSet TemplateTypeParmTypes;
   mutable llvm::FoldingSet ObjCTypeParamTypes;
   mutable llvm::FoldingSet
@@ -238,7 +243,8 @@ class ASTContext : public RefCountedBase {
   mutable llvm::FoldingSet AttributedTypes;
   mutable llvm::FoldingSet PipeTypes;
   mutable llvm::FoldingSet BitIntTypes;
-  mutable llvm::FoldingSet DependentBitIntTypes;
+  mutable llvm::ContextualFoldingSet
+  DependentBitIntTypes;
   llvm::FoldingSet BTFTagAttributedTypes;
 
   mutable llvm::FoldingSet QualifiedTemplateNames;
diff --git a/clang/include/clang/AST/Type.h b/clang/include/clang/AST/Type.h
index 4799f89db82fa7f..a78d8f60462b231 100644
--- a/clang/include/clang/AST/Type.h
+++ b/clang/include/clang/AST/Type.h
@@ -3289,8 +3289,6 @@ class VariableArrayType : public ArrayType {
 class DependentSizedArrayType : public ArrayType {
   friend class ASTContext; // ASTContext creates these.
 
-  const ASTContext &Context;
-
   /// An assignment expression that will instantiate to the
   /// size of the array.
   ///
@@ -3301,8 +3299,8 @@ class DependentSizedArrayType : public ArrayType {
   /// The range spanned by the left and right array brackets.
   SourceRange Brackets;
 
-  DependentSizedArrayType(const ASTContext &Context, QualType et, QualType can,
-  Expr *e, ArraySizeModifier sm, unsigned tq,
+  DependentSizedArrayType(QualType et, QualType can, Expr *e,
+  ArraySizeModifier sm, unsigned tq,
   SourceRange brackets);
 
 public:
@@ -3325,7 +3323,7 @@ class DependentSizedArrayType : public ArrayType {
 return T->getTypeClass() == DependentSizedArray;
   }
 
-  void Profile(llvm::FoldingSetNodeID &ID) {
+  void Profile(llvm::FoldingSetNodeID &ID, const ASTContext &Context) {
 Profile(ID, Context, getElementType(),
 getSizeModifier(), getIndexTypeCVRQualifiers(), getSizeExpr());
   }
@@ -3349,14 +3347,12 @@ class DependentSizedArrayType : public ArrayType {
 class DependentAddressSpaceType : public Type, public llvm::FoldingSetNode {
   friend class ASTContext;
 
-  const ASTContext &Context;
   Expr *AddrSpaceExpr;
   QualType PointeeType;
   SourceLocation loc;
 
-  DependentAddressSpaceType(const ASTContext &Context, QualType PointeeTy

[clang] [NFC][clang] change remaining context-dependent type nodes to ContextualFoldingSet (PR #67751)

[Matheus Izvekov via cfe-commits]

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

>From 5919cbf2a91f1cff3e48ba10cd2e9f5d2f5023f0 Mon Sep 17 00:00:00 2001
From: Matheus Izvekov 
Date: Fri, 29 Sep 2023 00:49:16 +0200
Subject: [PATCH] [NFC][clang] change remaining context-dependent type nodes to
 ContextualFoldingSet

---
 clang/include/clang/AST/ASTContext.h | 24 ++
 clang/include/clang/AST/Type.h   | 62 ++---
 clang/lib/AST/ASTContext.cpp | 68 
 clang/lib/AST/Type.cpp   | 47 +--
 4 files changed, 90 insertions(+), 111 deletions(-)

diff --git a/clang/include/clang/AST/ASTContext.h 
b/clang/include/clang/AST/ASTContext.h
index 4ee32c76a95d8e3..8ad0514ee2ce227 100644
--- a/clang/include/clang/AST/ASTContext.h
+++ b/clang/include/clang/AST/ASTContext.h
@@ -195,20 +195,25 @@ class ASTContext : public RefCountedBase {
   ConstantArrayTypes;
   mutable llvm::FoldingSet IncompleteArrayTypes;
   mutable std::vector VariableArrayTypes;
-  mutable llvm::FoldingSet DependentSizedArrayTypes;
-  mutable llvm::FoldingSet
-DependentSizedExtVectorTypes;
-  mutable llvm::FoldingSet
+  mutable llvm::ContextualFoldingSet
+  DependentSizedArrayTypes;
+  mutable llvm::ContextualFoldingSet
+  DependentSizedExtVectorTypes;
+  mutable llvm::ContextualFoldingSet
   DependentAddressSpaceTypes;
   mutable llvm::FoldingSet VectorTypes;
-  mutable llvm::FoldingSet DependentVectorTypes;
+  mutable llvm::ContextualFoldingSet
+  DependentVectorTypes;
   mutable llvm::FoldingSet MatrixTypes;
-  mutable llvm::FoldingSet DependentSizedMatrixTypes;
+  mutable llvm::ContextualFoldingSet
+  DependentSizedMatrixTypes;
   mutable llvm::FoldingSet FunctionNoProtoTypes;
   mutable llvm::ContextualFoldingSet
 FunctionProtoTypes;
-  mutable llvm::FoldingSet DependentTypeOfExprTypes;
-  mutable llvm::FoldingSet DependentDecltypeTypes;
+  mutable llvm::ContextualFoldingSet
+  DependentTypeOfExprTypes;
+  mutable llvm::ContextualFoldingSet
+  DependentDecltypeTypes;
   mutable llvm::FoldingSet TemplateTypeParmTypes;
   mutable llvm::FoldingSet ObjCTypeParamTypes;
   mutable llvm::FoldingSet
@@ -238,7 +243,8 @@ class ASTContext : public RefCountedBase {
   mutable llvm::FoldingSet AttributedTypes;
   mutable llvm::FoldingSet PipeTypes;
   mutable llvm::FoldingSet BitIntTypes;
-  mutable llvm::FoldingSet DependentBitIntTypes;
+  mutable llvm::ContextualFoldingSet
+  DependentBitIntTypes;
   llvm::FoldingSet BTFTagAttributedTypes;
 
   mutable llvm::FoldingSet QualifiedTemplateNames;
diff --git a/clang/include/clang/AST/Type.h b/clang/include/clang/AST/Type.h
index 4799f89db82fa7f..a78d8f60462b231 100644
--- a/clang/include/clang/AST/Type.h
+++ b/clang/include/clang/AST/Type.h
@@ -3289,8 +3289,6 @@ class VariableArrayType : public ArrayType {
 class DependentSizedArrayType : public ArrayType {
   friend class ASTContext; // ASTContext creates these.
 
-  const ASTContext &Context;
-
   /// An assignment expression that will instantiate to the
   /// size of the array.
   ///
@@ -3301,8 +3299,8 @@ class DependentSizedArrayType : public ArrayType {
   /// The range spanned by the left and right array brackets.
   SourceRange Brackets;
 
-  DependentSizedArrayType(const ASTContext &Context, QualType et, QualType can,
-  Expr *e, ArraySizeModifier sm, unsigned tq,
+  DependentSizedArrayType(QualType et, QualType can, Expr *e,
+  ArraySizeModifier sm, unsigned tq,
   SourceRange brackets);
 
 public:
@@ -3325,7 +3323,7 @@ class DependentSizedArrayType : public ArrayType {
 return T->getTypeClass() == DependentSizedArray;
   }
 
-  void Profile(llvm::FoldingSetNodeID &ID) {
+  void Profile(llvm::FoldingSetNodeID &ID, const ASTContext &Context) {
 Profile(ID, Context, getElementType(),
 getSizeModifier(), getIndexTypeCVRQualifiers(), getSizeExpr());
   }
@@ -3349,14 +3347,12 @@ class DependentSizedArrayType : public ArrayType {
 class DependentAddressSpaceType : public Type, public llvm::FoldingSetNode {
   friend class ASTContext;
 
-  const ASTContext &Context;
   Expr *AddrSpaceExpr;
   QualType PointeeType;
   SourceLocation loc;
 
-  DependentAddressSpaceType(const ASTContext &Context, QualType PointeeType,
-QualType can, Expr *AddrSpaceExpr,
-SourceLocation loc);
+  DependentAddressSpaceType(QualType PointeeType, QualType can,
+Expr *AddrSpaceExpr, SourceLocation loc);
 
 public:
   Expr *getAddrSpaceExpr() const { return AddrSpaceExpr; }
@@ -3370,7 +3366,7 @@ class DependentAddressSpaceType : public Type, public 
llvm::FoldingSetNode {
 return T->getTypeClass() == DependentAddressSpace;
   }
 
-  void Profile(llvm::FoldingSetNodeID &ID) {
+  void Profile(llvm::FoldingSetNodeID &ID, const ASTContext &Context) {
 Profile(ID, Context, g

[clang] [NFC][clang] change remaining context-dependent type nodes to ContextualFoldingSet (PR #67751)

[Matheus Izvekov via cfe-commits]

https://github.com/mizvekov edited 
https://github.com/llvm/llvm-project/pull/67751
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [NFC][clang] change remaining context-dependent type nodes to ContextualFoldingSet (PR #67751)

[Shafik Yaghmour via cfe-commits]

shafik wrote:

Thank you for the PR. Could you provide some context in the PR description? 
From the title it sounds like there were prior changes that this is finishing. 
It would be helpful to have a link to those prior changes.

https://github.com/llvm/llvm-project/pull/67751
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [NFC][clang] change remaining context-dependent type nodes to ContextualFoldingSet (PR #67751)

[Matheus Izvekov via cfe-commits]

mizvekov wrote:

Thanks for the review.

I think the change stands on itself, as we are avoiding storing one pointer per 
Type Node instance, for the cost of one extra pointer per ASTContext, which is 
negligible.

These type nodes we are changing here are old, at least as far back as 2008.
In 2010 this `ContextualFoldingSet` folding set was added in this commit: 
b9639aaed4ef98becf413b9d52680686f8f22ca1
To solve exactly this problem as you can see in the commit description.

But we never went back and updated the existing users. This patch does that.

https://github.com/llvm/llvm-project/pull/67751
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [NFC][clang] change remaining context-dependent type nodes to ContextualFoldingSet (PR #67751)

[Matheus Izvekov via cfe-commits]

https://github.com/mizvekov edited 
https://github.com/llvm/llvm-project/pull/67751
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [NFC][clang] change remaining context-dependent type nodes to ContextualFoldingSet (PR #67751)

[Matheus Izvekov via cfe-commits]

https://github.com/mizvekov edited 
https://github.com/llvm/llvm-project/pull/67751
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [NFC][clang] change remaining context-dependent type nodes to ContextualFoldingSet (PR #67751)

[Shafik Yaghmour via cfe-commits]

shafik wrote:

Thank you for improving the description. The description is usually what ends 
up in the git log and so it is important for that summary there to be as 
helpful and descriptive as possible. 

https://github.com/llvm/llvm-project/pull/67751
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [NFC][clang] change remaining context-dependent type nodes to ContextualFoldingSet (PR #67751)

[Matheus Izvekov via cfe-commits]

mizvekov wrote:

@shafik from the last message, it seemed like this is good to go, but you 
didn't approve, so I am wondering if you forgot.

https://github.com/llvm/llvm-project/pull/67751
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [NFC][clang] change remaining context-dependent type nodes to ContextualFoldingSet (PR #67751)

[via cfe-commits]

https://github.com/cor3ntin approved this pull request.

LGTM, thanks!

https://github.com/llvm/llvm-project/pull/67751
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [NFC][clang] change remaining context-dependent type nodes to ContextualFoldingSet (PR #67751)

[Matheus Izvekov via cfe-commits]

https://github.com/mizvekov closed 
https://github.com/llvm/llvm-project/pull/67751
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits