Skip to content

[clang] implement common sugared type of inst-dependent DecltypeType #67739

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Sep 29, 2023

Conversation

mizvekov
Copy link
Contributor

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 #67603

@mizvekov mizvekov self-assigned this Sep 28, 2023
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Sep 28, 2023
@llvmbot
Copy link
Member

llvmbot commented Sep 28, 2023

@llvm/pr-subscribers-clang

Changes

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 #67603


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

2 Files Affected:

  • (modified) clang/lib/AST/ASTContext.cpp (+8-1)
  • (modified) clang/test/SemaCXX/sugar-common-types.cpp (+11)
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<DecltypeType>(X), *DY = cast<DecltypeType>(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<DependentNameType>(X),
                *NY = cast<DependentNameType>(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<id, id> p;
   id t = false ? p.first : p.second;
 } // namespace PR61419
+
+namespace GH67603 {
+  template <class> using A = long;
+  template <class B> void h() {
+    using C = B;
+    using D = B;
+    N t = 0 ? A<decltype(C())>() : A<decltype(D())>();
+    // expected-error@-1 {{rvalue of type 'A<decltype(C())>' (aka 'long')}}
+  }
+  template void h<int>();
+} // namespace GH67603

Copy link
Collaborator

@shafik shafik left a comment

Choose a reason for hiding this comment

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

Thank you for the quick fix.

LGTM, please add a release note.

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 llvm#67603
@mizvekov mizvekov force-pushed the fix_decltype_commonsugar branch from e6c6a38 to 7009ebf Compare September 28, 2023 21:39
@mizvekov mizvekov merged commit 06721bb into llvm:main Sep 29, 2023
@mizvekov mizvekov deleted the fix_decltype_commonsugar branch September 29, 2023 00:22
legrosbuffle pushed a commit to legrosbuffle/llvm-project that referenced this pull request Sep 29, 2023
…lvm#67739)

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 llvm#67603
ahatanaka pushed a commit to swiftlang/llvm-project that referenced this pull request Dec 15, 2023
…lvm#67739)

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 llvm#67603

(cherry picked from commit 06721bb)

Conflicts:
	clang/docs/ReleaseNotes.rst
ahatanaka pushed a commit to swiftlang/llvm-project that referenced this pull request Dec 15, 2023
…lvm#67739)

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 llvm#67603

(cherry picked from commit 06721bb)

Conflicts:
	clang/docs/ReleaseNotes.rst
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[clang++][Front-end bugs] UNREACHABLE executed at /root/llvm-project/clang/lib/AST/ASTContext.cpp:12692
3 participants