[clang] Fix missing diagnostic of declaration use when accessing TypeDecls through typename access #129681

mizvekov · 2025-03-04T10:40:14Z

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.

Fixes #58547

Differential Revision: https://reviews.llvm.org/D136533

llvmbot · 2025-03-04T10:40:46Z

@llvm/pr-subscribers-clang

Author: Matheus Izvekov (mizvekov)

Changes

Fixes GH58547.

Differential Revision: https://reviews.llvm.org/D136533

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

5 Files Affected:

(modified) clang/docs/ReleaseNotes.rst (+3-4)
(modified) clang/include/clang/Sema/Sema.h (+4)
(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)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 08fafa00d5452..840660cb1c371 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.
+  `Issue 58547 <https://github.com/llvm/llvm-project/issues/58547>`
 
 C/C++ Language Potentially Breaking Changes
 -------------------------------------------
@@ -96,10 +99,6 @@ Resolutions to C++ Defect Reports
 C Language Changes
 ------------------
 
-- Clang now allows an ``inline`` specifier on a typedef declaration of a
-  function type in Microsoft compatibility mode. #GH124869
-- Clang now allows ``restrict`` qualifier for array types with pointer elements (#GH92847).
-
 C2y Feature Support
 ^^^^^^^^^^^^^^^^^^^
 
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 5b5cee5810488..40cab377ec347 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -3168,6 +3168,10 @@ class Sema final : public SemaBase {
 
   DeclGroupPtrTy ConvertDeclToDeclGroup(Decl *Ptr, Decl *OwnedType = nullptr);
 
+  enum class DiagCtorKind { None, Implicit, Typename };
+  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/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index fe313c62ff846..64126304c3f1c 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -137,6 +137,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,
@@ -293,10 +313,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;
@@ -320,11 +341,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);
@@ -513,18 +532,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 38fa3ff3ab5b4..fcbbf5dbffa53 100644
--- a/clang/lib/Sema/SemaTemplate.cpp
+++ b/clang/lib/Sema/SemaTemplate.cpp
@@ -10914,20 +10914,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

zyn0217

Sorry I didn't quite follow the discussion on phab, but do we still have issues with libc++ at this point?

mizvekov · 2025-03-04T12:24:10Z

No, at the time, I didn't have a MacOS machine to test this, and no one followed up to help track the problem.

Now that I have such a machine, I don't see the issue anymore.

zyn0217

Thanks for picking it up. I haven't dived into it yet, so let me ask if I understand the issue correctly: did we previously miss a call to DiagnoseUseOfDecl() in Sema::CheckTypenameType()? Otherwise, this looks good to me assuming there are no problems on macOS.

clang/include/clang/Sema/Sema.h

clang/docs/ReleaseNotes.rst

mizvekov · 2025-03-04T13:49:47Z

Thanks for picking it up. I haven't dived into it yet, so let me ask if I understand the issue correctly: did we previously miss a call to DiagnoseUseOfDecl() in Sema::CheckTypenameType()? Otherwise, this looks good to me assuming there are no problems on macOS.

Yeah, missing check. This refactors the code so this bug is more unlikely to resurface.

mizvekov · 2025-03-04T13:52:47Z

When I posted the PR, I had run the libc++ tests locally on a MacOS machine and this all passes.

…Decls through typename access Fixes #58547 Differential Revision: https://reviews.llvm.org/D136533

erichkeane

Shafik is going to ask for more details in the commit message :) Could you better describe the problem there and how you fixed it?

zmodem · 2025-03-10T12:46:46Z

We're seeing new -Wunguarded-availability warnings after this. A reduced example:

$ cat /tmp/x.cc
template<class T> struct remove_cv { typedef T type; };

struct __attribute__((__availability__(android, introduced = 29))) AAudioStreamWrapper { };

template <typename T> struct Foo {
  T *p;
};

template <typename T> struct Bar {
  typename remove_cv<T>::type *p;
};


struct __attribute__((__availability__(android, introduced = 29))) User {
  AAudioStreamWrapper a; // okay
  Foo<AAudioStreamWrapper> b; // okay

  Bar<AAudioStreamWrapper> c; // not okay?
};

$ build/bin/clang -c -target aarch64-linux-android26 -Wunguarded-availability /tmp/x.cc
/tmp/x.cc:10:26: warning: 'AAudioStreamWrapper' is only available on Android 29 or newer [-Wunguarded-availability]
   10 |   typename remove_cv<T>::type *p;
      |                          ^
/tmp/x.cc:18:28: note: in instantiation of template class 'Bar<AAudioStreamWrapper>' requested here
   18 |   Bar<AAudioStreamWrapper> c; // not okay?
      |                            ^
/tmp/x.cc:3:68: note: 'AAudioStreamWrapper' has been marked as being introduced in Android 29 here, but the deployment target is Android 26
    3 | struct __attribute__((__availability__(android, introduced = 29))) AAudioStreamWrapper { };
      |                                                                    ^
/tmp/x.cc:9:30: note: annotate 'Bar<AAudioStreamWrapper>' with an availability attribute to silence this warning
    9 | template <typename T> struct Bar {
      |                              ^
1 warning generated.

I believe the new warning is not correct, since c (like the other members) is in fact guarded by the availability attribute.

I'll revert to green for now until this can be investigated.

…ing TypeDecls through typename access (#129681)" This caused incorrect -Wunguarded-availability warnings. See comment on the pull request. > 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. > > Fixes #58547 > > Differential Revision: https://reviews.llvm.org/D136533 This reverts commit 4c4fd6b.

mizvekov · 2025-03-10T13:46:24Z

@zmodem Alright, I think this may have been the original bug, but you are the first to step in with a reproducer, thank you very much!

…Decls through typename access 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 Differential Revision: https://reviews.llvm.org/D136533

…Decls through typename access (llvm#129681) 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. Fixes llvm#58547 Differential Revision: https://reviews.llvm.org/D136533

mizvekov requested review from zygoloid, cor3ntin, erichkeane and zyn0217 March 4, 2025 10:40

mizvekov self-assigned this Mar 4, 2025

llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Mar 4, 2025

mizvekov force-pushed the users/mizvekov/GH58547 branch 3 times, most recently from 72b41d1 to 2ac8fd8 Compare March 4, 2025 10:47

zyn0217 reviewed Mar 4, 2025

View reviewed changes

zyn0217 approved these changes Mar 4, 2025

View reviewed changes

clang/include/clang/Sema/Sema.h Show resolved Hide resolved

clang/docs/ReleaseNotes.rst Outdated Show resolved Hide resolved

[clang] Fix missing diagnostic of declaration use when accessing Type…

1bc38b1

…Decls through typename access Fixes #58547 Differential Revision: https://reviews.llvm.org/D136533

mizvekov force-pushed the users/mizvekov/GH58547 branch from 2ac8fd8 to 1bc38b1 Compare March 4, 2025 14:22

erichkeane approved these changes Mar 4, 2025

View reviewed changes

mizvekov merged commit 4c4fd6b into main Mar 4, 2025
12 checks passed

mizvekov deleted the users/mizvekov/GH58547 branch March 4, 2025 15:15

shiltian mentioned this pull request Mar 10, 2025

[AMDGPU] Fix test failures when expensive checks are enabled #130644

Merged

mizvekov mentioned this pull request Mar 10, 2025

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

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[clang] Fix missing diagnostic of declaration use when accessing TypeDecls through typename access #129681

[clang] Fix missing diagnostic of declaration use when accessing TypeDecls through typename access #129681

Uh oh!

mizvekov commented Mar 4, 2025 •

edited

Loading

Uh oh!

llvmbot commented Mar 4, 2025

Uh oh!

zyn0217 left a comment

Uh oh!

mizvekov commented Mar 4, 2025

Uh oh!

zyn0217 left a comment

Uh oh!

Uh oh!

Uh oh!

mizvekov commented Mar 4, 2025

Uh oh!

mizvekov commented Mar 4, 2025

Uh oh!

erichkeane left a comment

Uh oh!

Uh oh!

zmodem commented Mar 10, 2025

Uh oh!

mizvekov commented Mar 10, 2025

Uh oh!

Uh oh!

[clang] Fix missing diagnostic of declaration use when accessing TypeDecls through typename access #129681

[clang] Fix missing diagnostic of declaration use when accessing TypeDecls through typename access #129681

Uh oh!

Conversation

mizvekov commented Mar 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

llvmbot commented Mar 4, 2025

Uh oh!

zyn0217 left a comment

Choose a reason for hiding this comment

Uh oh!

mizvekov commented Mar 4, 2025

Uh oh!

zyn0217 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

mizvekov commented Mar 4, 2025

Uh oh!

mizvekov commented Mar 4, 2025

Uh oh!

erichkeane left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

zmodem commented Mar 10, 2025

Uh oh!

mizvekov commented Mar 10, 2025

Uh oh!

Uh oh!

mizvekov commented Mar 4, 2025 •

edited

Loading