Call MarkVirtualMembersReferenced on an actual class definition

[sberg]

Summary

...rather than on potentially just a declaration.
Without the fix, the newly added clang/test/SemaCXX/warn-undefined-internal.cpp failed with

error: 'warning' diagnostics expected but not seen:
  File /home/sbergman/github.com/llvm/llvm-project/clang/test/SemaCXX/warn-undefined-internal.cpp Line 12 (directive at /home/sbergman/github.com/llvm/llvm-project/clang/test/SemaCXX/warn-undefined-internal.cpp:13): function 'test2()::S::f' has internal linkage but is not defined
error: 'note' diagnostics expected but not seen:
  File /home/sbergman/github.com/llvm/llvm-project/clang/test/SemaCXX/warn-undefined-internal.cpp Line 14 (directive at /home/sbergman/github.com/llvm/llvm-project/clang/test/SemaCXX/warn-undefined-internal.cpp:15): used here

(I ran into this when two LibreOffice Clang plugins produced false positive warnings, as they relied on Decl::isReferenced() returning true for such virtual member functions of local classes.)

[sberg]

clang/lib/Sema/SemaDeclCXX.cpp

17940

That call of getCanonicalDecl originated with https://github.com/llvm/llvm-project/commit/88d292ccb86c10857298c252bb93331f7ef2258a "Rework when and how vtables are emitted, by tracking where vtables are", and I wonder if more of the uses of the modified Class below suffer from the same issue, that they expect Class to represent a definition when it might potentially reference just a declaration.

[aaron.ballman]

Thanks! LGTM, though please add a release note about the fix.

clang/lib/Sema/SemaDeclCXX.cpp

17940

The only other place the class is being used is in VTableUses and Sema::DefineUsedVTables() does CXXRecordDecl *Class = VTableUses[I].first->getDefinition(); so I think that's safe (if it a bit convoluted). So I think this is okay as-is.

clang/docs/ReleaseNotes.rst

 Improvements to Clang's diagnostics
 -----------------------------------
 - We now generate a diagnostic for signed integer overflow due to unary minus
   in a non-constant expression context.
   (`#31643 <https://github.com/llvm/llvm-project/issues/31643>`)
 - Clang now warns by default for C++20 and later about deprecated capture of
   ``this`` with a capture default of ``=``. This warning can be disabled with
   ``-Wno-deprecated-this-capture``.
+- Clang had failed to emit some ``-Wundefined-internal`` for members of a local
+  class if that class was first introduced with a forward declaration.
 
 Bug Fixes in This Version
 -------------------------
 
 - Fix crash when diagnosing incorrect usage of ``_Nullable`` involving alias
   templates.
   (`#60344 <https://github.com/llvm/llvm-project/issues/60344>`_)
 - Fix confusing warning message when ``/clang:-x`` is passed in ``clang-cl``

clang/lib/Sema/SemaDeclCXX.cpp

if (TUKind != TU_Prefix && LangOpts.OpenMP && LangOpts.OpenMPIsDevice &&
       !isInOpenMPTargetExecutionDirective()) {
     if (!DefinitionRequired)
       MarkVirtualMembersReferenced(Loc, Class);
     return;
   }
 
   // Try to insert this class into the map.
   LoadExternalVTableUses();
   Class = Class->getCanonicalDecl();
   std::pair<llvm::DenseMap<CXXRecordDecl *, bool>::iterator, bool>
     Pos = VTablesUsed.insert(std::make_pair(Class, DefinitionRequired));
   if (!Pos.second) {
     // If we already had an entry, check to see if we are promoting this vtable
     // to require a definition. If so, we need to reappend to the VTableUses
     // list, since we may have already processed the first entry.
     if (DefinitionRequired && !Pos.first->second) {
       Pos.first->second = true;
if (Context.getTargetInfo().getCXXABI().isMicrosoft()) {
       }
     }
   }
 
   // Local classes need to have their virtual members marked
   // immediately. For all other classes, we mark their virtual members
   // at the end of the translation unit.
   if (Class->isLocalClass())
-    MarkVirtualMembersReferenced(Loc, Class);
+    MarkVirtualMembersReferenced(Loc, Class->getDefinition());
   else
     VTableUses.push_back(std::make_pair(Class, Loc));
 }
 
 bool Sema::DefineUsedVTables() {
   LoadExternalVTableUses();
   if (VTableUses.empty())
     return false;

clang/test/SemaCXX/warn-undefined-internal.cpp

+// RUN: %clang_cc1 -fsyntax-only -Wundefined-internal -verify %s
+
+void test1() {
+  struct S { virtual void f(); };
+  // expected-warning@-1{{function 'test1()::S::f' has internal linkage but is not defined}}
+  S s;
+  // expected-note@-1{{used here}}
+}
+
+void test2() {
+  struct S;
+  struct S { virtual void f(); };
+  // expected-warning@-1{{function 'test2()::S::f' has internal linkage but is not defined}}
+  S s;
+  // expected-note@-1{{used here}}
+}