Skip to content

Conversation

@mizvekov
Copy link
Contributor

@mizvekov mizvekov commented Apr 9, 2025

A NestedNameSpecifier of TypeSpec kind can be non-dependent even if its prefix is dependent, when for example the prefix is an injected class type but the type itself is a simple alias to a non-dependent type.

This issue was a bit hard to observe because if it is an alias to a class type, then we (for some unknown reason) ignored that the NNS was dependent in the first place, which wouldn't happen with an enum type.

This could have been a workaround for previous dependency bugs, and is not relevant anymore for any of the test cases in the tree, so this patch also removes that.

The other kinds of dependencies are still relevant. If the prefix contains an unexpanded pack, then this NNS is still unexpanded, and likewise for errors.

This fixes a regression reported here: #133610 (comment) which was introduced by #133610

There are no release notes since the regression was never released.

A NestedNameSpecifier of TypeSpec kind can be non-dependent even if its
prefix is dependent, when for example the prefix is an injected class type
but the type itself is a simple alias to a non-dependent type.

This issue was a bit hard to observe because if its an alias to
a class type, then we (for some unknown reason) ignored that the
NNS was dependent in the first place, which wouldn't happen
with an enum type.

This could have been a workaround for previous dependency bugs,
and is not relevant anymore for any of the test cases in the
tree, so this patch also removes that.

The other kinds of dependencies are still relevant.
If the prefix contains an unexpanded pack, then this NNS is still
unexpanded, and likewise for errors.

This fixes a regression reported here: #133610 (comment)
which was introduced by #133610

There are no release notes since the regression was never released.
@mizvekov mizvekov self-assigned this Apr 9, 2025
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Apr 9, 2025
@llvmbot
Copy link
Member

llvmbot commented Apr 9, 2025

@llvm/pr-subscribers-clang

Author: Matheus Izvekov (mizvekov)

Changes

A NestedNameSpecifier of TypeSpec kind can be non-dependent even if its prefix is dependent, when for example the prefix is an injected class type but the type itself is a simple alias to a non-dependent type.

This issue was a bit hard to observe because if its an alias to a class type, then we (for some unknown reason) ignored that the NNS was dependent in the first place, which wouldn't happen with an enum type.

This could have been a workaround for previous dependency bugs, and is not relevant anymore for any of the test cases in the tree, so this patch also removes that.

The other kinds of dependencies are still relevant. If the prefix contains an unexpanded pack, then this NNS is still unexpanded, and likewise for errors.

This fixes a regression reported here: #133610 (comment) which was introduced by #133610

There are no release notes since the regression was never released.


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

3 Files Affected:

  • (modified) clang/lib/AST/NestedNameSpecifier.cpp (+2-1)
  • (modified) clang/lib/Sema/SemaCXXScopeSpec.cpp (+1-2)
  • (modified) clang/test/SemaTemplate/dependent-names.cpp (+14)
diff --git a/clang/lib/AST/NestedNameSpecifier.cpp b/clang/lib/AST/NestedNameSpecifier.cpp
index 51aa2d69d0f0d..bd0fe36d781da 100644
--- a/clang/lib/AST/NestedNameSpecifier.cpp
+++ b/clang/lib/AST/NestedNameSpecifier.cpp
@@ -221,7 +221,8 @@ NestedNameSpecifierDependence NestedNameSpecifier::getDependence() const {
     NestedNameSpecifierDependence Dep =
         toNestedNameSpecifierDependendence(getAsType()->getDependence());
     if (NestedNameSpecifier *Prefix = getPrefix())
-      Dep |= Prefix->getDependence();
+      Dep |=
+          Prefix->getDependence() & ~NestedNameSpecifierDependence::Dependent;
     return Dep;
   }
   }
diff --git a/clang/lib/Sema/SemaCXXScopeSpec.cpp b/clang/lib/Sema/SemaCXXScopeSpec.cpp
index ea7936b0e5b8f..68ee006c6cf00 100644
--- a/clang/lib/Sema/SemaCXXScopeSpec.cpp
+++ b/clang/lib/Sema/SemaCXXScopeSpec.cpp
@@ -31,8 +31,7 @@ static CXXRecordDecl *getCurrentInstantiationOf(QualType T,
   const Type *Ty = T->getCanonicalTypeInternal().getTypePtr();
   if (const RecordType *RecordTy = dyn_cast<RecordType>(Ty)) {
     CXXRecordDecl *Record = cast<CXXRecordDecl>(RecordTy->getDecl());
-    if (!Record->isDependentContext() ||
-        Record->isCurrentInstantiation(CurContext))
+    if (Record->isCurrentInstantiation(CurContext))
       return Record;
 
     return nullptr;
diff --git a/clang/test/SemaTemplate/dependent-names.cpp b/clang/test/SemaTemplate/dependent-names.cpp
index 538abde3eddd5..54ce3760d2d01 100644
--- a/clang/test/SemaTemplate/dependent-names.cpp
+++ b/clang/test/SemaTemplate/dependent-names.cpp
@@ -467,3 +467,17 @@ namespace TransformDependentTemplates {
     void f(Arg<int>);
   };
 } // namespace TransformDependentTemplates
+
+namespace TransformNestedName {
+  enum class S { kA };
+
+  template <class T> struct N {
+    using State = S;
+    template <typename T::template X<State::kA> = 0>
+    void F();
+  };
+
+  template <class T>
+  template <typename T::template X<N<T>::State::kA>>
+  inline void N<T>::F() {}
+} // namespace TransformNestedName

@mizvekov mizvekov merged commit 154507c into main Apr 9, 2025
14 checks passed
@mizvekov mizvekov deleted the users/mizvekov/fix-nns-dependency branch April 9, 2025 22:44
AllinLeeYL pushed a commit to AllinLeeYL/llvm-project that referenced this pull request Apr 10, 2025
A NestedNameSpecifier of TypeSpec kind can be non-dependent even if its
prefix is dependent, when for example the prefix is an injected class
type but the type itself is a simple alias to a non-dependent type.

This issue was a bit hard to observe because if it is an alias to a
class type, then we (for some unknown reason) ignored that the NNS was
dependent in the first place, which wouldn't happen with an enum type.

This could have been a workaround for previous dependency bugs, and is
not relevant anymore for any of the test cases in the tree, so this
patch also removes that.

The other kinds of dependencies are still relevant. If the prefix
contains an unexpanded pack, then this NNS is still unexpanded, and
likewise for errors.

This fixes a regression reported here:
llvm#133610 (comment)
which was introduced by llvm#133610

There are no release notes since the regression was never released.
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