[clang] Fix wrong -Wunused-local-typedef warning within a template function

[krisb]

Summary

Partially fixes PR24883.

The patch sets Reference bit while instantiating a typedef if it
previously was found referenced.

[krisb]

clang/test/Modules/odr_hash.cpp

4375

I'm not sure what was the original intent of this test (i.e. whether it intentionally tests the fact that there is no error on an uninstantiated static member function). As well as it doesn't clear to me what is the role of the unused typedef here, but it starts triggering the error because of redecls() call on isReferenced() added by this patch.

[krisb]

Ping

[rtrieu]

clang/test/Modules/odr_hash.cpp

4375

I've checked out the ODR Hashing versus the other cases. The new error is in line with the other behavior. Having this new error is okay.

[krisb]

@rtrieu, thank you for looking at this! Do you have any comments about the rest of the patch? Do you think it makes sense?

clang/test/Modules/odr_hash.cpp

4375

Thank you for checking this!

[krisb]

Ping

[krisb]

Ping

[thakis]

Seems like a progression :)

Do you have commit permissoins?

[• Quuxplusone]

clang/test/SemaCXX/warn-unused-local-typedef.cpp

246

I haven't tried to understand the main point of this PR, but FWIW, have you seen https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61596 ? This looks suspiciously like that exact case. (I just filed its dup https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104792 the other day.)

[krisb]

clang/test/SemaCXX/warn-unused-local-typedef.cpp

246

This patch prevents clang from warning on used typedefs (Int case), if those typedefs are defined within a local class of a templated function or a member function of a templated class.
For Char case, which is unused in this example, the patch changes nothing.

Regarding https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104792, those typedefs seem well known, so could we just make clang skipping any checks for those particular names?
Alternatively, we would likely need to stop warning on any typedefs defined within a function-local class (but those warnings may still be useful like in the test from this patch).

[krisb]

clang/test/SemaCXX/warn-unused-local-typedef.cpp

246

@Quuxplusone if you do not have objections for this patch, I'd land it now. It'll make things a bit better and do not prevent us from relaxing conditions for the warning further.

thanks for your post! It helps me so much!
five nights at freddy's

clang/lib/Sema/SemaTemplateInstantiateDecl.cpp

Decl *TemplateDeclInstantiator::InstantiateTypedefNameDecl(TypedefNameDecl *D,
   }
 
   SemaRef.InstantiateAttrs(TemplateArgs, D, Typedef);
 
   if (D->getUnderlyingType()->getAs<DependentNameType>())
     SemaRef.inferGslPointerAttribute(Typedef);
 
   Typedef->setAccess(D->getAccess());
+  Typedef->setReferenced(D->isReferenced());
 
   return Typedef;
 }
 
 Decl *TemplateDeclInstantiator::VisitTypedefDecl(TypedefDecl *D) {
   Decl *Typedef = InstantiateTypedefNameDecl(D, /*IsTypeAlias=*/false);
   if (Typedef)
     Owner->addDecl(Typedef);

clang/test/AST/ast-dump-openmp-begin-declare-variant_reference.cpp

 // CHECK-NEXT: | |       `-DeclRefExpr [[ADDR_41:0x[a-z0-9]*]] <col:30> '_Tp' {{.*}}ParmVar [[ADDR_34]] '__t' '_Tp &&'
 // CHECK-NEXT: | |-FunctionDecl [[ADDR_42:0x[a-z0-9]*]] <line:10:1, line:14:1> line:11:1 used move 'typename remove_reference<float &>::type &&(float &)' inline
 // CHECK-NEXT: | | |-TemplateArgument type 'float &'
 // CHECK-NEXT: | | | `-LValueReferenceType [[ADDR_7]] 'float &'
 // CHECK-NEXT: | | |   `-BuiltinType [[ADDR_8]] 'float'
 // CHECK-NEXT: | | |-ParmVarDecl [[ADDR_43:0x[a-z0-9]*]] <col:6, col:12> col:12 used __t 'float &'
 // CHECK-NEXT: | | `-CompoundStmt [[ADDR_44:0x[a-z0-9]*]] <col:17, line:14:1>
 // CHECK-NEXT: | |   |-DeclStmt [[ADDR_45:0x[a-z0-9]*]] <line:12:3, col:51>
-// CHECK-NEXT: | |   | `-TypedefDecl [[ADDR_46:0x[a-z0-9]*]] <col:3, col:48> col:48 _Up 'typename remove_reference<float &>::type':'float'
+// CHECK-NEXT: | |   | `-TypedefDecl [[ADDR_46:0x[a-z0-9]*]] <col:3, col:48> col:48 referenced _Up 'typename remove_reference<float &>::type':'float'
 // CHECK-NEXT: | |   |   `-ElaboratedType [[ADDR_47:0x[a-z0-9]*]] 'typename remove_reference<float &>::type' sugar
 // CHECK-NEXT: | |   |     `-TypedefType [[ADDR_48:0x[a-z0-9]*]] 'remove_reference<float &>::type' sugar
 // CHECK-NEXT: | |   |       |-Typedef [[ADDR_10]] 'type'
 // CHECK-NEXT: | |   |       `-SubstTemplateTypeParmType [[ADDR_11]] 'float' sugar
 // CHECK-NEXT: | |   |         |-TemplateTypeParmType [[ADDR_12]] '_Tp' dependent depth 0 index 0
 // CHECK-NEXT: | |   |         | `-TemplateTypeParm [[ADDR_13]] '_Tp'
 // CHECK-NEXT: | |   |         `-BuiltinType [[ADDR_8]] 'float'
 // CHECK-NEXT: | |   `-ReturnStmt [[ADDR_49:0x[a-z0-9]*]] <line:13:3, col:33>
 // CHECK-NEXT: | |     `-CXXStaticCastExpr [[ADDR_50:0x[a-z0-9]*]] <col:10, col:33> '_Up':'float' xvalue static_cast<_Up &&> <NoOp>
 // CHECK-NEXT: | |       `-DeclRefExpr [[ADDR_51:0x[a-z0-9]*]] <col:30> 'float' {{.*}}ParmVar [[ADDR_43]] '__t' 'float &'
 // CHECK-NEXT: | `-FunctionDecl [[ADDR_52:0x[a-z0-9]*]] <line:10:1, line:14:1> line:11:1 used move 'typename remove_reference<short &>::type &&(short &)' inline
 // CHECK-NEXT: |   |-TemplateArgument type 'short &'
 // CHECK-NEXT: |   | `-LValueReferenceType [[ADDR_15]] 'short &'
 // CHECK-NEXT: |   |   `-BuiltinType [[ADDR_16]] 'short'
 // CHECK-NEXT: |   |-ParmVarDecl [[ADDR_53:0x[a-z0-9]*]] <col:6, col:12> col:12 used __t 'short &'
 // CHECK-NEXT: |   `-CompoundStmt [[ADDR_54:0x[a-z0-9]*]] <col:17, line:14:1>
 // CHECK-NEXT: |     |-DeclStmt [[ADDR_55:0x[a-z0-9]*]] <line:12:3, col:51>
-// CHECK-NEXT: |     | `-TypedefDecl [[ADDR_56:0x[a-z0-9]*]] <col:3, col:48> col:48 _Up 'typename remove_reference<short &>::type':'short'
+// CHECK-NEXT: |     | `-TypedefDecl [[ADDR_56:0x[a-z0-9]*]] <col:3, col:48> col:48 referenced _Up 'typename remove_reference<short &>::type':'short'
 // CHECK-NEXT: |     |   `-ElaboratedType [[ADDR_57:0x[a-z0-9]*]] 'typename remove_reference<short &>::type' sugar
 // CHECK-NEXT: |     |     `-TypedefType [[ADDR_58:0x[a-z0-9]*]] 'remove_reference<short &>::type' sugar
 // CHECK-NEXT: |     |       |-Typedef [[ADDR_18]] 'type'
 // CHECK-NEXT: |     |       `-SubstTemplateTypeParmType [[ADDR_19]] 'short' sugar
 // CHECK-NEXT: |     |         |-TemplateTypeParmType [[ADDR_12]] '_Tp' dependent depth 0 index 0
 // CHECK-NEXT: |     |         | `-TemplateTypeParm [[ADDR_13]] '_Tp'
 // CHECK-NEXT: |     |         `-BuiltinType [[ADDR_16]] 'short'
 // CHECK-NEXT: |     `-ReturnStmt [[ADDR_59:0x[a-z0-9]*]] <line:13:3, col:33>

clang/test/Modules/odr_hash.cpp

class S {
    typedef G Type;
    static inline G *Foo(const G *a, int * = nullptr);
 };
 
 template<typename G>
 G* S<G>::Foo(const G* asdf, int*) {}
 #else
 S<X> s;
+// expected-error@first.h:* {{'ParameterTest::S::Foo' has different definitions in different modules; definition in module 'FirstModule' first difference is 1st parameter with name 'aaaa'}}
+// expected-note@second.h:* {{but in 'SecondModule' found 1st parameter with name 'asdf'}}
 #endif
 }  // ParameterTest
 
 namespace MultipleTypedefs {
 #if defined(FIRST)
 typedef int B1;
 typedef B1 A1;
 struct S1 {

clang/test/SemaCXX/warn-unused-local-typedef.cpp

void placement_new_and_delete() {
 
   typedef MyStruct A_t1;
   MyStruct *a = new (p) A_t1();
 
   typedef MyStruct A_t2;
   a->~A_t2();
 }
 
+namespace TypedefInLocalClassOfAMemberOfTemplateClass {
+template<typename> struct A {
+  void foo() {
+    struct Inner {
+      typedef int Int; // no-diag
+      typedef char Char; // expected-warning {{unused typedef 'Char'}}
+      Int m;
+    } b;
+  }
+};
+
+void foo() {
+  A<int> x;
+  x.foo();
+}
+} // TypedefInLocalClassOfTemplateClassMember
+
 // This should not disable any warnings:
 #pragma clang diagnostic ignored "-Wunused-local-typedef"