Skip to content
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

[clang] Substitute alias templates from correct context #74335

Merged
merged 2 commits into from
Dec 6, 2023

Conversation

Fznamznon
Copy link
Contributor

Current context set to where alias was met, not where it is declared caused incorrect access check in case alias referenced private members of the parent class.

Fixes #41693

Current context set to where alias was met, not where it
is declared caused incorrect access check in case alias referenced
private members of the parent class.

Fixes llvm#41693
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Dec 4, 2023
@llvmbot
Copy link
Collaborator

llvmbot commented Dec 4, 2023

@llvm/pr-subscribers-clang

Author: Mariya Podchishchaeva (Fznamznon)

Changes

Current context set to where alias was met, not where it is declared caused incorrect access check in case alias referenced private members of the parent class.

Fixes #41693


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

5 Files Affected:

  • (modified) clang/docs/ReleaseNotes.rst (+3)
  • (modified) clang/lib/Sema/SemaCXXScopeSpec.cpp (+20-3)
  • (modified) clang/lib/Sema/SemaTemplate.cpp (+8-3)
  • (modified) clang/test/CXX/temp/temp.decls/temp.alias/p3.cpp (+3-2)
  • (modified) clang/test/SemaCXX/alias-template.cpp (+50)
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 683d0026bb345..18a7afda3c8b0 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -653,6 +653,9 @@ Bug Fixes in This Version
   Fixes (`#13826 <https://github.com/llvm/llvm-project/issues/13826>`_)
 - Clang's ``-Wchar-subscripts`` no longer warns on chars whose values are known non-negative constants.
   Fixes (`#18763 <https://github.com/llvm/llvm-project/issues/18763>`_)
+- Fixed false positive error emitted when templated alias inside a class
+  used private members of the same class.
+  Fixes (`#41693 <https://github.com/llvm/llvm-project/issues/41693>`_)
 
 Bug Fixes to Compiler Builtins
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
diff --git a/clang/lib/Sema/SemaCXXScopeSpec.cpp b/clang/lib/Sema/SemaCXXScopeSpec.cpp
index 44a40215b90df..56f83be78266f 100644
--- a/clang/lib/Sema/SemaCXXScopeSpec.cpp
+++ b/clang/lib/Sema/SemaCXXScopeSpec.cpp
@@ -30,6 +30,21 @@ static CXXRecordDecl *getCurrentInstantiationOf(QualType T,
     return nullptr;
 
   const Type *Ty = T->getCanonicalTypeInternal().getTypePtr();
+  if (isa<TemplateSpecializationType>(Ty)) {
+    if (auto *Record = dyn_cast<CXXRecordDecl>(CurContext)) {
+      if (isa<ClassTemplatePartialSpecializationDecl>(Record) ||
+          Record->getDescribedClassTemplate()) {
+        const Type *ICNT = Record->getTypeForDecl();
+        QualType Injected =
+            cast<InjectedClassNameType>(ICNT)->getInjectedSpecializationType();
+
+        if (Ty == Injected->getCanonicalTypeInternal().getTypePtr()) {
+          return Record;
+        }
+      }
+    }
+  }
+
   if (const RecordType *RecordTy = dyn_cast<RecordType>(Ty)) {
     CXXRecordDecl *Record = cast<CXXRecordDecl>(RecordTy->getDecl());
     if (!Record->isDependentContext() ||
@@ -37,10 +52,12 @@ static CXXRecordDecl *getCurrentInstantiationOf(QualType T,
       return Record;
 
     return nullptr;
-  } else if (isa<InjectedClassNameType>(Ty))
+  }
+
+  if (isa<InjectedClassNameType>(Ty))
     return cast<InjectedClassNameType>(Ty)->getDecl();
-  else
-    return nullptr;
+
+  return nullptr;
 }
 
 /// Compute the DeclContext that is associated with the given type.
diff --git a/clang/lib/Sema/SemaTemplate.cpp b/clang/lib/Sema/SemaTemplate.cpp
index 09bbf14d39af5..23a175da6bc4d 100644
--- a/clang/lib/Sema/SemaTemplate.cpp
+++ b/clang/lib/Sema/SemaTemplate.cpp
@@ -3990,9 +3990,14 @@ QualType Sema::CheckTemplateIdType(TemplateName Name,
     if (Inst.isInvalid())
       return QualType();
 
-    CanonType = SubstType(Pattern->getUnderlyingType(),
-                          TemplateArgLists, AliasTemplate->getLocation(),
-                          AliasTemplate->getDeclName());
+    bool ForLambdaCallOperator = false;
+    if (const auto *Rec = dyn_cast<CXXRecordDecl>(Pattern->getDeclContext()))
+      ForLambdaCallOperator = Rec->isLambda();
+    Sema::ContextRAII SavedContext(*this, Pattern->getDeclContext(),
+                                   !ForLambdaCallOperator);
+    CanonType =
+        SubstType(Pattern->getUnderlyingType(), TemplateArgLists,
+                  AliasTemplate->getLocation(), AliasTemplate->getDeclName());
     if (CanonType.isNull()) {
       // If this was enable_if and we failed to find the nested type
       // within enable_if in a SFINAE context, dig out the specific
diff --git a/clang/test/CXX/temp/temp.decls/temp.alias/p3.cpp b/clang/test/CXX/temp/temp.decls/temp.alias/p3.cpp
index 2d46502e1d9b3..2b33a4ef566da 100644
--- a/clang/test/CXX/temp/temp.decls/temp.alias/p3.cpp
+++ b/clang/test/CXX/temp/temp.decls/temp.alias/p3.cpp
@@ -2,11 +2,12 @@
 
 // The example given in the standard (this is rejected for other reasons anyway).
 template<class T> struct A;
-template<class T> using B = typename A<T>::U; // expected-error {{no type named 'U' in 'A<T>'}}
+template<class T> using B = typename A<T>::U; // expected-error {{no type named 'U' in 'A<short>'}}
+                                              // expected-note@-1 {{in instantiation of template class 'A<short>' requested here}}
 template<class T> struct A {
   typedef B<T> U; // expected-note {{in instantiation of template type alias 'B' requested here}}
 };
-B<short> b;
+B<short> b; // expected-note {{in instantiation of template type alias 'B' requested here}}
 
 template<typename T> using U = int;
 
diff --git a/clang/test/SemaCXX/alias-template.cpp b/clang/test/SemaCXX/alias-template.cpp
index 5189405e23db5..cd415c6cd4e9b 100644
--- a/clang/test/SemaCXX/alias-template.cpp
+++ b/clang/test/SemaCXX/alias-template.cpp
@@ -192,3 +192,53 @@ int g = sfinae_me<int>(); // expected-error{{no matching function for call to 's
 namespace NullExceptionDecl {
 template<int... I> auto get = []() { try { } catch(...) {}; return I; }; // expected-error{{initializer contains unexpanded parameter pack 'I'}}
 }
+
+namespace GH41693 {
+struct S {
+private:
+  template <typename> static constexpr void Impl() {}
+
+public:
+  template <typename X> using U = decltype(Impl<X>());
+};
+
+using X = S::U<void>;
+struct Y {
+private:
+    static constexpr int x=0;
+
+    template <typename>
+    static constexpr int y=0;
+
+    template <typename>
+    static constexpr int foo();
+
+public:
+    template <typename U>
+    using bar1 = decltype(foo<U>());
+    using bar2 = decltype(x);
+    template <typename U>
+    using bar3 = decltype(y<U>);
+};
+
+
+using type1 = Y::bar1<float>;
+using type2 = Y::bar2;
+using type3 = Y::bar3<float>;
+
+struct theFriend{
+    template<class T>
+    using theAlias = decltype(&T::i);
+};
+
+class theC{
+  int i;
+  public:
+  friend struct theFriend;
+};
+
+int foo(){
+  (void)sizeof(theFriend::theAlias<theC>);
+}
+
+}

Copy link
Collaborator

@erichkeane erichkeane left a comment

Choose a reason for hiding this comment

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

I think this is on the right track, 1 nit, else is right I think.

clang/lib/Sema/SemaTemplate.cpp Outdated Show resolved Hide resolved
@Fznamznon Fznamznon merged commit 6b1aa31 into llvm:main Dec 6, 2023
4 checks passed
@metaflow
Copy link
Contributor

metaflow commented Dec 6, 2023

This started to give errors to code previously considere valid with "out-of-line definition of X differs from that in".
Don't have a repro yet.

@erichkeane
Copy link
Collaborator

This started to give errors to code previously considere valid with "out-of-line definition of X differs from that in". Don't have a repro yet.

Please provide one as soon as you can! In the meantime, we should probably revert @Fznamznon

@Fznamznon
Copy link
Contributor Author

Please provide one as soon as you can! In the meantime, we should probably revert @Fznamznon

Ok, I'll prepare revert.

Fznamznon added a commit that referenced this pull request Dec 6, 2023
…)"

It was reported in the PR that commit caused clang giving errors for
code previously considered valid.
This reverts commit 6b1aa31.
@Fznamznon
Copy link
Contributor Author

Ok, revert is done.
@metaflow , please provide the reproducer.

@metaflow
Copy link
Contributor

metaflow commented Dec 6, 2023

here is a sample for OSS ducc https://gitlab.mpcdf.mpg.de/mtr/ducc.git (lines might be off)

In file included from third_party/ducc/src/ducc0/fft/fftnd_impl.h:76:
third_party/ducc/src/ducc0/fft/fft1d_impl.h:1730:51: error: return type of out-of-line definition of 'ducc0::detail_fft::cfftpass::make_pass' differs from that in the declaration
 1730 | template<typename Tfs> Tcpass<Tfs> cfftpass<Tfs>::make_pass(size_t l1,
      |                        ~~~~~~~~~~~                ^
third_party/ducc/src/ducc0/fft/fft.h:205:33: note: previous declaration is here
  205 |     static shared_ptr<cfftpass> make_pass(size_t l1, size_t ido, size_t ip,
      |            ~~~~~~~~~~~~~~~~~~~~ ^
In file included from third_party/ducc/src/ducc0/fft/fftnd_impl.h:76:
third_party/ducc/src/ducc0/fft/fft1d_impl.h:2945:51: error: return type of out-of-line definition of 'ducc0::detail_fft::rfftpass::make_pass' differs from that in the declaration
 2945 | template<typename Tfs> Trpass<Tfs> rfftpass<Tfs>::make_pass(size_t l1,
      |                        ~~~~~~~~~~~                ^
third_party/ducc/src/ducc0/fft/fft.h:249:33: note: previous declaration is here
  249 |     static shared_ptr<rfftpass> make_pass(size_t l1, size_t ido, size_t ip,
      |            ~~~~~~~~~~~~~~~~~~~~ ^

sorry I don't have a smaller repro. It seems that "using" in the class body somehow messes things up.

@Fznamznon
Copy link
Contributor Author

Thanks, I reproduced the problem with


template <typename T> class shared_ptr {
    T* data;
};

template <typename Tfs> class cfftpass {
    static shared_ptr<cfftpass> make_pass();
};

template<typename T> using Tcpass = shared_ptr<cfftpass<T>>;

template<typename Tfs> Tcpass<Tfs> cfftpass<Tfs>::make_pass() { return Tcpass<Tfs>{};};                                                                   

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.

Invalid private member diagnostic of template using with decltype of private member method
4 participants