Reland: [clang] Fix missing diagnostic of declaration use when accessing TypeDecls through typename access

[mizvekov]

We were missing a call to DiagnoseUseOfDecl when performing typename access.

This refactors the code so that TypeDecl lookups funnel through a helper which performs all the necessary checks, removing some related duplication on the way.

When diagnosing availability uses through typedefs, this changes the implementation so it won't dig through subst nodes, as those uses should be diagnosed through the template specialization it was accessed from. This takes care of the regression reported here: #129681 (comment)

Fixes #58547

Original PR: #129681
Differential Revision: https://reviews.llvm.org/D136533

[llvmbot]

@llvm/pr-subscribers-clang

Author: Matheus Izvekov (mizvekov)

Changes

We were missing a call to DiagnoseUseOfDecl when performing typename access.

This refactors the code so that TypeDecl lookups funnel through a helper which performs all the necessary checks, removing some related duplication on the way.

When diagnosing availability uses through typedefs, this changes the implementation so it won't dig through subst nodes, as those uses should be diagnosed through the template specialization it was accessed from. This takes care of the regression reported here: #129681 (comment)

Fixes #58547

Original PR: #129681
Differential Revision: https://reviews.llvm.org/D136533

---

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

7 Files Affected:

• (modified) clang/docs/ReleaseNotes.rst (+3)
• (modified) clang/include/clang/Sema/Sema.h (+7)
• (modified) clang/lib/Sema/SemaAvailability.cpp (+18-5)
• (modified) clang/lib/Sema/SemaDecl.cpp (+29-18)
• (modified) clang/lib/Sema/SemaTemplate.cpp (+7-12)
• (modified) clang/test/CXX/dcl.dcl/dcl.attr/dcl.attr.deprecated/p1.cpp (+14)
• (modified) clang/test/Sema/attr-availability-macosx.cpp (+14-1)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 01b018857a7fc..d123a3d010361 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -35,6 +35,9 @@ Potentially Breaking Changes
 ============================
 
 - The Objective-C ARC migrator (ARCMigrate) has been removed.
+- Fix missing diagnostics for uses of declarations when performing typename access,
+  such as when performing member access on a '[[deprecated]]' type alias.
+  (#GH58547)
 
 C/C++ Language Potentially Breaking Changes
 -------------------------------------------
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 9a312e1c55f60..9ac26d8728446 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -3168,6 +3168,13 @@ class Sema final : public SemaBase {
 
   DeclGroupPtrTy ConvertDeclToDeclGroup(Decl *Ptr, Decl *OwnedType = nullptr);
 
+  enum class DiagCtorKind { None, Implicit, Typename };
+  /// Returns the TypeDeclType for the given type declaration,
+  /// as ASTContext::getTypeDeclType would, but
+  /// performs the required semantic checks for name lookup of said entity.
+  QualType getTypeDeclType(DeclContext *LookupCtx, DiagCtorKind DCK,
+                           TypeDecl *TD, SourceLocation NameLoc);
+
   /// If the identifier refers to a type name within this scope,
   /// return the declaration of that type.
   ///
diff --git a/clang/lib/Sema/SemaAvailability.cpp b/clang/lib/Sema/SemaAvailability.cpp
index c806b832dec7a..96aa65412906c 100644
--- a/clang/lib/Sema/SemaAvailability.cpp
+++ b/clang/lib/Sema/SemaAvailability.cpp
@@ -99,16 +99,29 @@ ShouldDiagnoseAvailabilityOfDecl(Sema &S, const NamedDecl *D,
   // For typedefs, if the typedef declaration appears available look
   // to the underlying type to see if it is more restrictive.
   while (const auto *TD = dyn_cast<TypedefNameDecl>(D)) {
-    if (Result == AR_Available) {
-      if (const auto *TT = TD->getUnderlyingType()->getAs<TagType>()) {
+    if (Result != AR_Available)
+      break;
+    for (const Type *T = TD->getUnderlyingType().getTypePtr(); /**/; /**/) {
+      if (auto *TT = dyn_cast<TagType>(T)) {
         D = TT->getDecl();
-        Result = D->getAvailability(Message);
+      } else if (isa<SubstTemplateTypeParmType>(T)) {
+        // A Subst* node represents a use through a template.
+        // Any uses of the underlying declaration happened through it's template
+        // specialization.
+        goto done;
+      } else {
+        const Type *NextT =
+            T->getLocallyUnqualifiedSingleStepDesugaredType().getTypePtr();
+        if (NextT == T)
+          goto done;
+        T = NextT;
         continue;
       }
+      Result = D->getAvailability(Message);
+      break;
     }
-    break;
   }
-
+done:
   // For alias templates, get the underlying declaration.
   if (const auto *ADecl = dyn_cast<TypeAliasTemplateDecl>(D)) {
     D = ADecl->getTemplatedDecl();
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index dcb419fdd6f1b..9c67fbd40ac71 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -139,6 +139,26 @@ class TypeNameValidatorCCC final : public CorrectionCandidateCallback {
 
 } // end anonymous namespace
 
+QualType Sema::getTypeDeclType(DeclContext *LookupCtx, DiagCtorKind DCK,
+                               TypeDecl *TD, SourceLocation NameLoc) {
+  auto *LookupRD = dyn_cast_or_null<CXXRecordDecl>(LookupCtx);
+  auto *FoundRD = dyn_cast<CXXRecordDecl>(TD);
+  if (DCK != DiagCtorKind::None && LookupRD && FoundRD &&
+      FoundRD->isInjectedClassName() &&
+      declaresSameEntity(LookupRD, cast<Decl>(FoundRD->getParent()))) {
+    Diag(NameLoc,
+         DCK == DiagCtorKind::Typename
+             ? diag::ext_out_of_line_qualified_id_type_names_constructor
+             : diag::err_out_of_line_qualified_id_type_names_constructor)
+        << TD->getIdentifier() << /*Type=*/1
+        << 0 /*if any keyword was present, it was 'typename'*/;
+  }
+
+  DiagnoseUseOfDecl(TD, NameLoc);
+  MarkAnyDeclReferenced(TD->getLocation(), TD, /*OdrUse=*/false);
+  return Context.getTypeDeclType(TD);
+}
+
 namespace {
 enum class UnqualifiedTypeNameLookupResult {
   NotFound,
@@ -295,10 +315,11 @@ ParsedType Sema::getTypeName(const IdentifierInfo &II, SourceLocation NameLoc,
                              bool IsClassTemplateDeductionContext,
                              ImplicitTypenameContext AllowImplicitTypename,
                              IdentifierInfo **CorrectedII) {
+  bool IsImplicitTypename = !isClassName && !IsCtorOrDtorName;
   // FIXME: Consider allowing this outside C++1z mode as an extension.
   bool AllowDeducedTemplate = IsClassTemplateDeductionContext &&
-                              getLangOpts().CPlusPlus17 && !IsCtorOrDtorName &&
-                              !isClassName && !HasTrailingDot;
+                              getLangOpts().CPlusPlus17 && IsImplicitTypename &&
+                              !HasTrailingDot;
 
   // Determine where we will perform name lookup.
   DeclContext *LookupCtx = nullptr;
@@ -322,11 +343,9 @@ ParsedType Sema::getTypeName(const IdentifierInfo &II, SourceLocation NameLoc,
         // refer to a member of an unknown specialization.
         // In C++2a, in several contexts a 'typename' is not required. Also
         // allow this as an extension.
-        if (AllowImplicitTypename == ImplicitTypenameContext::No &&
-            !isClassName && !IsCtorOrDtorName)
-          return nullptr;
-        bool IsImplicitTypename = !isClassName && !IsCtorOrDtorName;
         if (IsImplicitTypename) {
+          if (AllowImplicitTypename == ImplicitTypenameContext::No)
+            return nullptr;
           SourceLocation QualifiedLoc = SS->getRange().getBegin();
           if (getLangOpts().CPlusPlus20)
             Diag(QualifiedLoc, diag::warn_cxx17_compat_implicit_typename);
@@ -515,18 +534,10 @@ ParsedType Sema::getTypeName(const IdentifierInfo &II, SourceLocation NameLoc,
     // C++ [class.qual]p2: A lookup that would find the injected-class-name
     // instead names the constructors of the class, except when naming a class.
     // This is ill-formed when we're not actually forming a ctor or dtor name.
-    auto *LookupRD = dyn_cast_or_null<CXXRecordDecl>(LookupCtx);
-    auto *FoundRD = dyn_cast<CXXRecordDecl>(TD);
-    if (!isClassName && !IsCtorOrDtorName && LookupRD && FoundRD &&
-        FoundRD->isInjectedClassName() &&
-        declaresSameEntity(LookupRD, cast<Decl>(FoundRD->getParent())))
-      Diag(NameLoc, diag::err_out_of_line_qualified_id_type_names_constructor)
-          << &II << /*Type*/1;
-
-    DiagnoseUseOfDecl(IIDecl, NameLoc);
-
-    T = Context.getTypeDeclType(TD);
-    MarkAnyDeclReferenced(TD->getLocation(), TD, /*OdrUse=*/false);
+    T = getTypeDeclType(LookupCtx,
+                        IsImplicitTypename ? DiagCtorKind::Implicit
+                                           : DiagCtorKind::None,
+                        TD, NameLoc);
   } else if (ObjCInterfaceDecl *IDecl = dyn_cast<ObjCInterfaceDecl>(IIDecl)) {
     (void)DiagnoseUseOfDecl(IDecl, NameLoc);
     if (!HasTrailingDot)
diff --git a/clang/lib/Sema/SemaTemplate.cpp b/clang/lib/Sema/SemaTemplate.cpp
index 290862d9a773d..64aabb1fcdc35 100644
--- a/clang/lib/Sema/SemaTemplate.cpp
+++ b/clang/lib/Sema/SemaTemplate.cpp
@@ -10930,20 +10930,15 @@ Sema::CheckTypenameType(ElaboratedTypeKeyword Keyword,
       //
       // FIXME: That's not strictly true: mem-initializer-id lookup does not
       // ignore functions, but that appears to be an oversight.
-      auto *LookupRD = dyn_cast_or_null<CXXRecordDecl>(Ctx);
-      auto *FoundRD = dyn_cast<CXXRecordDecl>(Type);
-      if (Keyword == ElaboratedTypeKeyword::Typename && LookupRD && FoundRD &&
-          FoundRD->isInjectedClassName() &&
-          declaresSameEntity(LookupRD, cast<Decl>(FoundRD->getParent())))
-        Diag(IILoc, diag::ext_out_of_line_qualified_id_type_names_constructor)
-            << &II << 1 << 0 /*'typename' keyword used*/;
-
+      QualType T = getTypeDeclType(Ctx,
+                                   Keyword == ElaboratedTypeKeyword::Typename
+                                       ? DiagCtorKind::Typename
+                                       : DiagCtorKind::None,
+                                   Type, IILoc);
       // We found a type. Build an ElaboratedType, since the
       // typename-specifier was just sugar.
-      MarkAnyDeclReferenced(Type->getLocation(), Type, /*OdrUse=*/false);
-      return Context.getElaboratedType(Keyword,
-                                       QualifierLoc.getNestedNameSpecifier(),
-                                       Context.getTypeDeclType(Type));
+      return Context.getElaboratedType(
+          Keyword, QualifierLoc.getNestedNameSpecifier(), T);
     }
 
     // C++ [dcl.type.simple]p2:
diff --git a/clang/test/CXX/dcl.dcl/dcl.attr/dcl.attr.deprecated/p1.cpp b/clang/test/CXX/dcl.dcl/dcl.attr/dcl.attr.deprecated/p1.cpp
index 58c7c0cfac8bf..26738583da506 100644
--- a/clang/test/CXX/dcl.dcl/dcl.attr/dcl.attr.deprecated/p1.cpp
+++ b/clang/test/CXX/dcl.dcl/dcl.attr/dcl.attr.deprecated/p1.cpp
@@ -58,3 +58,17 @@ template <typename T>
   FunS2 f;// No warning, entire function is deprecated, so usage here should be fine.
 
 }
+
+namespace GH58547 {
+struct A {
+  using ta [[deprecated]] = int; // expected-note 2{{marked deprecated here}}
+};
+
+using t1 = typename A::ta; // expected-warning {{'ta' is deprecated}}
+
+template <class B1> struct B {
+  using tb = typename B1::ta; // expected-warning {{'ta' is deprecated}}
+};
+
+template struct B<A>; // expected-note {{requested here}}
+} // namespace GH58547
diff --git a/clang/test/Sema/attr-availability-macosx.cpp b/clang/test/Sema/attr-availability-macosx.cpp
index 52f320d409281..41ac3e7495c43 100644
--- a/clang/test/Sema/attr-availability-macosx.cpp
+++ b/clang/test/Sema/attr-availability-macosx.cpp
@@ -14,4 +14,17 @@ bool try_acquire_for(T duration) { // expected-note{{'try_acquire_for<int>' has
 int main() {
   try_acquire_for(1); // expected-warning{{'try_acquire_for<int>' is only available on macOS 11 or newer}}
   // expected-note@-1{{enclose 'try_acquire_for<int>' in a __builtin_available check to silence this warning}}
-}
\ No newline at end of file
+}
+
+namespace typename_template {
+  struct [[clang::availability(macos, introduced = 16)]] A {};
+
+  template<class T> struct B { using type = T; };
+  template <class T> struct C {
+    typename B<T>::type v;
+  };
+
+  struct [[clang::availability(macos, introduced = 16)]] D {
+    C<A> c;
+  };
+} // namespace typename_template

[mizvekov]

This tests #129681 (comment)

[mizvekov]

This fixes #129681 (comment)

[zyn0217]

Do we need to take care of NTTP/template template parameters here?

[mizvekov]

NTTP it looks unlikely, as we would be needing to dig through expressions, which we don't here, but maybe elsewhere.

Template Template parameters maybe, but it looks far fetched for the kind of application this availability thing has.