Skip to content

[clang] Fix incorrect partial ordering context setting #108491

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 16, 2024

Conversation

mizvekov
Copy link
Contributor

Fixes regression introduced in #94981, reported on the pull-request.

Since this fixes a commit which was never released, there are no release notes.

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

llvmbot commented Sep 13, 2024

@llvm/pr-subscribers-clang

Author: Matheus Izvekov (mizvekov)

Changes

Fixes regression introduced in #94981, reported on the pull-request.

Since this fixes a commit which was never released, there are no release notes.


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

4 Files Affected:

  • (modified) clang/lib/Sema/SemaTemplateInstantiate.cpp (+5-3)
  • (modified) clang/lib/Sema/TreeTransform.h (+93-57)
  • (modified) clang/test/AST/ast-dump-template-name.cpp (+66-1)
  • (modified) clang/test/SemaTemplate/cwg2398.cpp (+14)
diff --git a/clang/lib/Sema/SemaTemplateInstantiate.cpp b/clang/lib/Sema/SemaTemplateInstantiate.cpp
index c42cc250bb904a..48ba7aae15268c 100644
--- a/clang/lib/Sema/SemaTemplateInstantiate.cpp
+++ b/clang/lib/Sema/SemaTemplateInstantiate.cpp
@@ -1973,9 +1973,11 @@ TemplateName TemplateInstantiator::TransformTemplateName(
     CXXScopeSpec &SS, TemplateName Name, SourceLocation NameLoc,
     QualType ObjectType, NamedDecl *FirstQualifierInScope,
     bool AllowInjectedClassName) {
-  if (TemplateTemplateParmDecl *TTP
-       = dyn_cast_or_null<TemplateTemplateParmDecl>(Name.getAsTemplateDecl())) {
-    if (TTP->getDepth() < TemplateArgs.getNumLevels()) {
+  // FIXME: Don't lose sugar here.
+  if (auto [TD, DefArgs] = Name.getTemplateDeclAndDefaultArgs();
+      TD && DefArgs.Args.empty()) {
+    if (auto *TTP = dyn_cast<TemplateTemplateParmDecl>(TD);
+        TTP && TTP->getDepth() < TemplateArgs.getNumLevels()) {
       // If the corresponding template argument is NULL or non-existent, it's
       // because we are performing instantiation from explicitly-specified
       // template arguments in a function template, but there were some
diff --git a/clang/lib/Sema/TreeTransform.h b/clang/lib/Sema/TreeTransform.h
index ff745b3385fcd9..b149cf13edc7b0 100644
--- a/clang/lib/Sema/TreeTransform.h
+++ b/clang/lib/Sema/TreeTransform.h
@@ -4540,6 +4540,63 @@ NestedNameSpecifierLoc TreeTransform<Derived>::TransformNestedNameSpecifierLoc(
   return SS.getWithLocInContext(SemaRef.Context);
 }
 
+/// Iterator adaptor that invents template argument location information
+/// for each of the template arguments in its underlying iterator.
+template <typename Derived, typename InputIterator>
+class TemplateArgumentLocInventIterator {
+  TreeTransform<Derived> &Self;
+  InputIterator Iter;
+
+public:
+  typedef TemplateArgumentLoc value_type;
+  typedef TemplateArgumentLoc reference;
+  typedef typename std::iterator_traits<InputIterator>::difference_type
+      difference_type;
+  typedef std::input_iterator_tag iterator_category;
+
+  class pointer {
+    TemplateArgumentLoc Arg;
+
+  public:
+    explicit pointer(TemplateArgumentLoc Arg) : Arg(Arg) {}
+
+    const TemplateArgumentLoc *operator->() const { return &Arg; }
+  };
+
+  explicit TemplateArgumentLocInventIterator(TreeTransform<Derived> &Self,
+                                             InputIterator Iter)
+      : Self(Self), Iter(Iter) {}
+
+  TemplateArgumentLocInventIterator &operator++() {
+    ++Iter;
+    return *this;
+  }
+
+  TemplateArgumentLocInventIterator operator++(int) {
+    TemplateArgumentLocInventIterator Old(*this);
+    ++(*this);
+    return Old;
+  }
+
+  reference operator*() const {
+    TemplateArgumentLoc Result;
+    Self.InventTemplateArgumentLoc(*Iter, Result);
+    return Result;
+  }
+
+  pointer operator->() const { return pointer(**this); }
+
+  friend bool operator==(const TemplateArgumentLocInventIterator &X,
+                         const TemplateArgumentLocInventIterator &Y) {
+    return X.Iter == Y.Iter;
+  }
+
+  friend bool operator!=(const TemplateArgumentLocInventIterator &X,
+                         const TemplateArgumentLocInventIterator &Y) {
+    return X.Iter != Y.Iter;
+  }
+};
+
 template<typename Derived>
 DeclarationNameInfo
 TreeTransform<Derived>
@@ -4661,6 +4718,42 @@ TreeTransform<Derived>::TransformTemplateName(CXXScopeSpec &SS,
                                             ObjectType, AllowInjectedClassName);
   }
 
+  if (DeducedTemplateStorage *DTN = Name.getAsDeducedTemplateName()) {
+    TemplateName Underlying = DTN->getUnderlying();
+    TemplateName TransUnderlying = getDerived().TransformTemplateName(
+        SS, Underlying, NameLoc, ObjectType, FirstQualifierInScope,
+        AllowInjectedClassName);
+    if (TransUnderlying.isNull())
+      return TemplateName();
+
+    DefaultArguments DefArgs = DTN->getDefaultArguments();
+
+    TemplateArgumentListInfo TransArgsInfo;
+    using Iterator =
+        TemplateArgumentLocInventIterator<Derived, TemplateArgument *>;
+    if (getDerived().TransformTemplateArguments(
+            Iterator(*this,
+                     const_cast<TemplateArgument *>(DefArgs.Args.begin())),
+            Iterator(*this, const_cast<TemplateArgument *>(DefArgs.Args.end())),
+            TransArgsInfo))
+      return TemplateName();
+
+    SmallVector<TemplateArgument, 4> TransArgs(
+        TransArgsInfo.arguments().size());
+    for (unsigned I = 0; I < TransArgs.size(); ++I)
+      TransArgs[I] = TransArgsInfo.arguments()[I].getArgument();
+
+    return getSema().Context.getDeducedTemplateName(
+        TransUnderlying, DefaultArguments{DefArgs.StartPos, TransArgs});
+  }
+
+  // FIXME: Preserve SubstTemplateTemplateParm.
+  if (SubstTemplateTemplateParmStorage *STN =
+          Name.getAsSubstTemplateTemplateParm())
+    return getDerived().TransformTemplateName(
+        SS, STN->getReplacement(), NameLoc, ObjectType, FirstQualifierInScope,
+        AllowInjectedClassName);
+
   // FIXME: Try to preserve more of the TemplateName.
   if (TemplateDecl *Template = Name.getAsTemplateDecl()) {
     TemplateDecl *TransTemplate
@@ -4807,63 +4900,6 @@ bool TreeTransform<Derived>::TransformTemplateArgument(
   return true;
 }
 
-/// Iterator adaptor that invents template argument location information
-/// for each of the template arguments in its underlying iterator.
-template<typename Derived, typename InputIterator>
-class TemplateArgumentLocInventIterator {
-  TreeTransform<Derived> &Self;
-  InputIterator Iter;
-
-public:
-  typedef TemplateArgumentLoc value_type;
-  typedef TemplateArgumentLoc reference;
-  typedef typename std::iterator_traits<InputIterator>::difference_type
-    difference_type;
-  typedef std::input_iterator_tag iterator_category;
-
-  class pointer {
-    TemplateArgumentLoc Arg;
-
-  public:
-    explicit pointer(TemplateArgumentLoc Arg) : Arg(Arg) { }
-
-    const TemplateArgumentLoc *operator->() const { return &Arg; }
-  };
-
-  explicit TemplateArgumentLocInventIterator(TreeTransform<Derived> &Self,
-                                             InputIterator Iter)
-    : Self(Self), Iter(Iter) { }
-
-  TemplateArgumentLocInventIterator &operator++() {
-    ++Iter;
-    return *this;
-  }
-
-  TemplateArgumentLocInventIterator operator++(int) {
-    TemplateArgumentLocInventIterator Old(*this);
-    ++(*this);
-    return Old;
-  }
-
-  reference operator*() const {
-    TemplateArgumentLoc Result;
-    Self.InventTemplateArgumentLoc(*Iter, Result);
-    return Result;
-  }
-
-  pointer operator->() const { return pointer(**this); }
-
-  friend bool operator==(const TemplateArgumentLocInventIterator &X,
-                         const TemplateArgumentLocInventIterator &Y) {
-    return X.Iter == Y.Iter;
-  }
-
-  friend bool operator!=(const TemplateArgumentLocInventIterator &X,
-                         const TemplateArgumentLocInventIterator &Y) {
-    return X.Iter != Y.Iter;
-  }
-};
-
 template<typename Derived>
 template<typename InputIterator>
 bool TreeTransform<Derived>::TransformTemplateArguments(
diff --git a/clang/test/AST/ast-dump-template-name.cpp b/clang/test/AST/ast-dump-template-name.cpp
index acacdac857954c..956191643546f0 100644
--- a/clang/test/AST/ast-dump-template-name.cpp
+++ b/clang/test/AST/ast-dump-template-name.cpp
@@ -1,4 +1,13 @@
-// RUN: %clang_cc1 -std=c++26 -ast-dump -ast-dump-filter=Test %s | FileCheck %s
+// Test without serialization:
+// RUN: %clang_cc1 -std=c++26 -triple x86_64-unknown-unknown -ast-dump -ast-dump-filter=Test %s \
+// RUN: | FileCheck -strict-whitespace %s
+//
+// Test with serialization:
+// RUN: %clang_cc1 -std=c++26 -triple x86_64-unknown-unknown -emit-pch -o %t %s
+// RUN: %clang_cc1 -x c++ -std=c++26 -triple x86_64-unknown-unknown -include-pch %t \
+// RUN:             -ast-dump-all -ast-dump-filter=Test /dev/null \
+// RUN: | sed -e "s/ <undeserialized declarations>//" -e "s/ imported//" \
+// RUN: | FileCheck --strict-whitespace %s
 
 template <template <class> class TT> using N = TT<int>;
 
@@ -58,3 +67,59 @@ namespace subst {
 // CHECK-NEXT:         | |-associated ClassTemplateSpecialization {{.+}} 'B'{{$}}
 // CHECK-NEXT:         | `-replacement:
 // CHECK-NEXT:         |   `-ClassTemplateDecl {{.+}} A{{$}}
+
+namespace deduced {
+  template <class> struct D;
+
+  template <class ET, template <class> class VT>
+  struct D<VT<ET>> {
+    using E = VT<char>;
+    template <class C> using F = VT<C>;
+  };
+
+  template <typename, int> class Matrix;
+
+  using TestDeduced1 = D<Matrix<double, 3>>::E;
+  using TestDeduced2 = D<Matrix<double, 3>>::F<int>;
+} // namespace deduced
+
+// CHECK:      Dumping deduced::TestDeduced1:
+// CHECK-NEXT: TypeAliasDecl
+// CHECK-NEXT: `-ElaboratedType
+// CHECK-NEXT:   `-TypedefType
+// CHECK-NEXT:     |-TypeAlias
+// CHECK-NEXT:     `-ElaboratedType
+// CHECK-NEXT:       `-TemplateSpecializationType
+// CHECK-NEXT:         |-name: 'deduced::Matrix:1<3>' subst index 1
+// CHECK-NEXT:         | |-parameter: TemplateTemplateParmDecl {{.+}} depth 0 index 1 VT
+// CHECK-NEXT:         | |-associated
+// CHECK-NEXT:         | `-replacement: 'deduced::Matrix:1<3>' deduced
+// CHECK-NEXT:         |   |-underlying: 'deduced::Matrix'
+// CHECK-NEXT:         |   | `-ClassTemplateDecl {{.+}} Matrix
+// CHECK-NEXT:         |   `-defaults:  start 1
+// CHECK-NEXT:         |     `-TemplateArgument integral '3'
+// CHECK-NEXT:         |-TemplateArgument type 'char'
+// CHECK-NEXT:         | `-BuiltinType
+// CHECK-NEXT:         `-RecordType
+// CHECK-NEXT:           `-ClassTemplateSpecialization
+
+// CHECK:      Dumping deduced::TestDeduced2:
+// CHECK-NEXT: TypeAliasDecl
+// CHECK-NEXT: `-ElaboratedType
+// CHECK-NEXT:   `-TemplateSpecializationType
+// CHECK-NEXT:     |-name: 'D<Matrix<double, 3>>::F':'deduced::D<deduced::Matrix<double, 3>>::F
+// CHECK-NEXT:     | |-NestedNameSpecifier
+// CHECK-NEXT:     | `-TypeAliasTemplateDecl
+// CHECK-NEXT:     |-TemplateArgument type 'int'
+// CHECK-NEXT:     | `-BuiltinType
+// CHECK-NEXT:     `-ElaboratedType
+// CHECK-NEXT:       `-TemplateSpecializationType
+// CHECK-NEXT:         |-name: 'Matrix:1<3>':'deduced::Matrix:1<3>' deduced
+// CHECK-NEXT:         | |-underlying: 'Matrix':'deduced::Matrix' qualified
+// CHECK-NEXT:         | | `-ClassTemplateDecl {{.+}} Matrix
+// CHECK-NEXT:         | `-defaults:  start 1
+// CHECK-NEXT:         |   `-TemplateArgument expr '3'
+// CHECK-NEXT:         |-TemplateArgument type 'int'
+// CHECK-NEXT:         | `-BuiltinType
+// CHECK-NEXT:         `-RecordType
+// CHECK-NEXT:           `-ClassTemplateSpecialization
diff --git a/clang/test/SemaTemplate/cwg2398.cpp b/clang/test/SemaTemplate/cwg2398.cpp
index 1d9747276fbe00..881c9dd2d697ff 100644
--- a/clang/test/SemaTemplate/cwg2398.cpp
+++ b/clang/test/SemaTemplate/cwg2398.cpp
@@ -379,3 +379,17 @@ namespace regression1 {
     bar(input);
   }
 } // namespace regression1
+
+namespace regression2 {
+  template <class> struct D;
+  // old-note@-1 {{template is declared here}}
+
+  template <class ET, template <class> class VT>
+  struct D<VT<ET>> {
+    template <class C> using E = VT<C>;
+  };
+
+  template <typename, int> class Matrix;
+  using X = D<Matrix<double, 3>>::E<int>;
+  // old-error@-1 {{implicit instantiation of undefined template}}
+} // namespace regression2

@alexfh
Copy link
Contributor

alexfh commented Sep 13, 2024

Unfortunately, the original code, from which I reduced the test case for this, still breaks (without crashing Clang though). I'm trying to come up with another test case now.

@alexfh
Copy link
Contributor

alexfh commented Sep 13, 2024

I could reduce the code to something that compiles with Clang before fa65804, but not after it, this PR doesn't fix the issue. The example is: https://gcc.godbolt.org/z/odWYhxGxK

Note that the code doesn't compile with Clang 18.1, but should compile with Clang 19 (which I couldn't find on Compiler Explorer) and for that matter with all Clang versions starting from b86e099 and up to fa65804.

Fixes regression introduced in #94981, reported on the pull-request.

Since this fixes a commit which was never released, there are no
release notes.
@mizvekov mizvekov force-pushed the users/mizvekov/clang-cwg2398-default-deduction-fix-1 branch from 2d79745 to d086374 Compare September 16, 2024 01:41
@mizvekov mizvekov changed the title [clang] Implement transforms for DeducedTemplateName [clang] Fix incorrect partial ordering context setting Sep 16, 2024
@mizvekov
Copy link
Contributor Author

@alexfh thanks, the latest push should fix it.

Copy link
Contributor

@alexfh alexfh left a comment

Choose a reason for hiding this comment

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

Thanks! This fixes the issues we've found so far, and it seems to be a quite clear correctness fix. LGTM

@mizvekov mizvekov merged commit 63b6c38 into main Sep 16, 2024
8 checks passed
@mizvekov mizvekov deleted the users/mizvekov/clang-cwg2398-default-deduction-fix-1 branch September 16, 2024 12:56
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.

4 participants