[PATCH] D114446: [clang] Fix wrong -Wunused-local-typedef warning if use is a dependent qialified identifier

2021-12-02 Thread Kristina Bessonova via Phabricator via cfe-commits
krisb updated this revision to Diff 391312.
krisb added a comment.

Simplified diagnostic condition in 
TemplateDeclInstantiator::InstantiateTypedefNameDecl().


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D114446/new/

https://reviews.llvm.org/D114446

Files:
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
  clang/test/SemaCXX/warn-unused-local-typedef.cpp


Index: clang/test/SemaCXX/warn-unused-local-typedef.cpp
===
--- clang/test/SemaCXX/warn-unused-local-typedef.cpp
+++ clang/test/SemaCXX/warn-unused-local-typedef.cpp
@@ -255,5 +255,20 @@
 }
 } // TypedefInLocalClassOfTemplateClassMember
 
+namespace TypedefInDependentQualifiedIdentifier {
+struct A { void f() const {} };
+
+template 
+void handler(const T ) {
+   using a_type_t = A; // no-diag
+  using b_type_t = A; // expected-warning {{unused type alias 'b_type_t'}}
+   item.a_type_t::f();
+}
+
+void foo() {
+   handler(A());
+}
+} // TypedefInDependentQualifiedIdentifier
+
 // This should not disable any warnings:
 #pragma clang diagnostic ignored "-Wunused-local-typedef"
Index: clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
===
--- clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -955,6 +955,10 @@
   Typedef->setAccess(D->getAccess());
   Typedef->setReferenced(D->isReferenced());
 
+  // Diagnose unused local typedefs.
+  if (!Typedef->isInvalidDecl() && (!RD || RD->isLocalClass()))
+SemaRef.DiagnoseUnusedDecl(Typedef);
+
   return Typedef;
 }
 
@@ -1907,8 +1911,6 @@
 LocalInstantiations.perform();
   }
 
-  SemaRef.DiagnoseUnusedNestedTypedefs(Record);
-
   if (IsInjectedClassName)
 assert(Record->isInjectedClassName() && "Broken injected-class-name");
 
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -1794,16 +1794,19 @@
 
   // Except for labels, we only care about unused decls that are local to
   // functions.
-  bool WithinFunction = D->getDeclContext()->isFunctionOrMethod();
-  if (const auto *R = dyn_cast(D->getDeclContext()))
-// For dependent types, the diagnostic is deferred.
-WithinFunction =
-WithinFunction || (R->isLocalClass() && !R->isDependentType());
+  auto *Context = D->getDeclContext();
+  bool WithinFunction = Context->isFunctionOrMethod();
+  if (const auto *R = dyn_cast(Context))
+WithinFunction = WithinFunction || R->isLocalClass();
   if (!WithinFunction)
 return false;
 
-  if (isa(D))
+  if (const auto *TD = dyn_cast(D)) {
+// Defer warnings for typedefs within a dependent context.
+if (Context->isDependentContext())
+  return false;
 return true;
+  }
 
   // White-list anything that isn't a local variable.
   if (!isa(D) || isa(D) || isa(D))


Index: clang/test/SemaCXX/warn-unused-local-typedef.cpp
===
--- clang/test/SemaCXX/warn-unused-local-typedef.cpp
+++ clang/test/SemaCXX/warn-unused-local-typedef.cpp
@@ -255,5 +255,20 @@
 }
 } // TypedefInLocalClassOfTemplateClassMember
 
+namespace TypedefInDependentQualifiedIdentifier {
+struct A { void f() const {} };
+
+template 
+void handler(const T ) {
+	using a_type_t = A; // no-diag
+  using b_type_t = A; // expected-warning {{unused type alias 'b_type_t'}}
+	item.a_type_t::f();
+}
+
+void foo() {
+	handler(A());
+}
+} // TypedefInDependentQualifiedIdentifier
+
 // This should not disable any warnings:
 #pragma clang diagnostic ignored "-Wunused-local-typedef"
Index: clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
===
--- clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -955,6 +955,10 @@
   Typedef->setAccess(D->getAccess());
   Typedef->setReferenced(D->isReferenced());
 
+  // Diagnose unused local typedefs.
+  if (!Typedef->isInvalidDecl() && (!RD || RD->isLocalClass()))
+SemaRef.DiagnoseUnusedDecl(Typedef);
+
   return Typedef;
 }
 
@@ -1907,8 +1911,6 @@
 LocalInstantiations.perform();
   }
 
-  SemaRef.DiagnoseUnusedNestedTypedefs(Record);
-
   if (IsInjectedClassName)
 assert(Record->isInjectedClassName() && "Broken injected-class-name");
 
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -1794,16 +1794,19 @@
 
   // Except for labels, we only care about unused decls that are local to
   // functions.
-  bool WithinFunction = D->getDeclContext()->isFunctionOrMethod();
-  if (const auto *R = dyn_cast(D->getDeclContext()))
-// For dependent types, the diagnostic is deferred.
-WithinFunction =
-

[PATCH] D114446: [clang] Fix wrong -Wunused-local-typedef warning if use is a dependent qialified identifier

2021-11-23 Thread Kristina Bessonova via Phabricator via cfe-commits
krisb created this revision.
krisb added reviewers: thakis, rtrieu, rsmith.
krisb requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Attempt to fix Tom Smeding's example from 
https://bugs.llvm.org/show_bug.cgi?id=24883.

Given the case like this one:

  struct A {
void f() const {}
  };
  
  template 
  void handler(const T ) {
using a_type_t = A;
item.a_type_t::f();
  }
  
  int main() {
handler(A());
  }

there is no way to know whether the typedef is used or not before
the templated context is instantiated.

Having this the patch proposes deffering all the diagnostics for
typedefs defined within a dependent context.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D114446

Files:
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
  clang/test/SemaCXX/warn-unused-local-typedef.cpp


Index: clang/test/SemaCXX/warn-unused-local-typedef.cpp
===
--- clang/test/SemaCXX/warn-unused-local-typedef.cpp
+++ clang/test/SemaCXX/warn-unused-local-typedef.cpp
@@ -255,5 +255,20 @@
 }
 } // TypedefInLocalClassOfTemplateClassMember
 
+namespace TypedefInDependentQualifiedIdentifier {
+struct A { void f() const {} };
+
+template 
+void handler(const T ) {
+   using a_type_t = A; // no-diag
+  using b_type_t = A; // expected-warning {{unused type alias 'b_type_t'}}
+   item.a_type_t::f();
+}
+
+void foo() {
+   handler(A());
+}
+} // TypedefInDependentQualifiedIdentifier
+
 // This should not disable any warnings:
 #pragma clang diagnostic ignored "-Wunused-local-typedef"
Index: clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
===
--- clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -955,6 +955,14 @@
   Typedef->setAccess(D->getAccess());
   Typedef->setReferenced(D->isReferenced());
 
+  // Diagnose unused local typedefs here since diagnostics for typedefs
+  // in a dependent context were deffered).
+  bool ShouldDiagnoseUnused = Typedef->getDeclContext()->isFunctionOrMethod();
+  if (const auto *R = dyn_cast(Typedef->getDeclContext()))
+ShouldDiagnoseUnused = ShouldDiagnoseUnused || R->isLocalClass();
+  if (!Typedef->isInvalidDecl() && ShouldDiagnoseUnused)
+SemaRef.DiagnoseUnusedDecl(Typedef);
+
   return Typedef;
 }
 
@@ -1907,8 +1915,6 @@
 LocalInstantiations.perform();
   }
 
-  SemaRef.DiagnoseUnusedNestedTypedefs(Record);
-
   if (IsInjectedClassName)
 assert(Record->isInjectedClassName() && "Broken injected-class-name");
 
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -1794,16 +1794,19 @@
 
   // Except for labels, we only care about unused decls that are local to
   // functions.
-  bool WithinFunction = D->getDeclContext()->isFunctionOrMethod();
-  if (const auto *R = dyn_cast(D->getDeclContext()))
-// For dependent types, the diagnostic is deferred.
-WithinFunction =
-WithinFunction || (R->isLocalClass() && !R->isDependentType());
+  auto *Context = D->getDeclContext();
+  bool WithinFunction = Context->isFunctionOrMethod();
+  if (const auto *R = dyn_cast(Context))
+WithinFunction = WithinFunction || R->isLocalClass();
   if (!WithinFunction)
 return false;
 
-  if (isa(D))
+  if (const auto *TD = dyn_cast(D)) {
+// Defer warnings for typedefs within a dependent context.
+if (Context->isDependentContext())
+  return false;
 return true;
+  }
 
   // White-list anything that isn't a local variable.
   if (!isa(D) || isa(D) || isa(D))


Index: clang/test/SemaCXX/warn-unused-local-typedef.cpp
===
--- clang/test/SemaCXX/warn-unused-local-typedef.cpp
+++ clang/test/SemaCXX/warn-unused-local-typedef.cpp
@@ -255,5 +255,20 @@
 }
 } // TypedefInLocalClassOfTemplateClassMember
 
+namespace TypedefInDependentQualifiedIdentifier {
+struct A { void f() const {} };
+
+template 
+void handler(const T ) {
+	using a_type_t = A; // no-diag
+  using b_type_t = A; // expected-warning {{unused type alias 'b_type_t'}}
+	item.a_type_t::f();
+}
+
+void foo() {
+	handler(A());
+}
+} // TypedefInDependentQualifiedIdentifier
+
 // This should not disable any warnings:
 #pragma clang diagnostic ignored "-Wunused-local-typedef"
Index: clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
===
--- clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -955,6 +955,14 @@
   Typedef->setAccess(D->getAccess());
   Typedef->setReferenced(D->isReferenced());
 
+  // Diagnose unused local typedefs here since diagnostics for typedefs
+  // in a dependent context were deffered).
+  bool