[clang-tidy] Fix performance-trivially-destructible with C++20 modules (llvm#178471)

When a class definition is seen through both a header include and a
C++20 module import, destructors may appear multiple times in the AST's
redeclaration chain. The original matcher used `isFirstDecl()` which
fails in this scenario because the same declaration can appear as both
first and non-first depending on the view.

Replace `unless(isFirstDecl())` with `isOutOfLine()` which correctly
identifies out-of-line definitions by checking whether the lexical
context differs from the semantic context.

Also update clang-tools-extra's lit.cfg.py to call `use_clang()` instead
of `clang_setup()` to make the `%clang` substitution available for
tests.

Fixes llvm#178102

Co-authored-by: Chuanqi Xu <yedeng.yd@linux.alibaba.com>

Author: Ignition

clang-tools-extra/clang-tidy/performance/TriviallyDestructibleCheck.cpp

@@ -20,7 +20,11 @@ namespace clang::tidy::performance {
 
 namespace {
 
-AST_MATCHER(Decl, isFirstDecl) { return Node.isFirstDecl(); }
+// We use isOutOfLine() to find out-of-line defaulted destructor definitions.
+// This is more robust than !isFirstDecl() because with C++20 modules, when a
+// class is visible through both a header include and a module import, its
+// declarations may appear multiple times in the redeclaration chain.
+AST_MATCHER(CXXMethodDecl, isOutOfLine) { return Node.isOutOfLine(); }
 
 AST_MATCHER_P(CXXRecordDecl, hasBase, Matcher<QualType>, InnerMatcher) {
   return llvm::any_of(Node.bases(), [&](const CXXBaseSpecifier &BaseSpec) {
@@ -33,8 +37,8 @@ AST_MATCHER_P(CXXRecordDecl, hasBase, Matcher<QualType>, InnerMatcher) {
 void TriviallyDestructibleCheck::registerMatchers(MatchFinder *Finder) {
   Finder->addMatcher(
       cxxDestructorDecl(
-          isDefaulted(),
-          unless(anyOf(isFirstDecl(), isVirtual(),
+          isDefaulted(), isOutOfLine(),
+          unless(anyOf(isVirtual(),
                        ofClass(cxxRecordDecl(
                            anyOf(hasBase(unless(isTriviallyDestructible())),
                                  has(fieldDecl(unless(

clang-tools-extra/docs/ReleaseNotes.rst

@@ -404,6 +404,11 @@ Changes in existing checks
 
   - Fixes false negatives when using ``std::set`` from ``libstdc++``.
 
+- Improved :doc:`performance-trivially-destructible
+  <clang-tidy/checks/performance/trivially-destructible>` check by fixing
+  false positives when a class is seen through both a header include and
+  a C++20 module import.
+
 - Improved :doc:`readability-container-size-empty
   <clang-tidy/checks/readability/container-size-empty>` check:
 

clang-tools-extra/test/CMakeLists.txt

@@ -44,6 +44,9 @@ set(CLANG_TOOLS_TEST_DEPS
   clang-resource-headers
 
   clang-tidy
+
+  # Some tests invoke clang directly (e.g., to precompile modules).
+  clang
 )
 
 # Add lit test dependencies.

clang-tools-extra/test/clang-tidy/checkers/performance/trivially-destructible-module.cpp

@@ -0,0 +1,61 @@
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: split-file %s %t
+// RUN: %clang -std=c++20 -x c++-module --precompile %t/mymodule.cppm -o %t/mymodule.pcm -I%t
+// RUN: %check_clang_tidy -std=c++20 %t/main.cpp performance-trivially-destructible %t/out \
+// RUN:     -- -- -std=c++20 -I%t -fmodule-file=mymodule=%t/mymodule.pcm
+
+// Test: C++20 modules - class visible through both #include and import.
+//
+// When a class is defined in a header that is both #include'd directly and
+// included in a module's global module fragment, the class's destructor may
+// appear multiple times in the redeclaration chain.
+//
+// This test verifies:
+// 1. No false positive on in-line defaulted destructor (struct A)
+// 2. No false positive on implicit destructor (struct X)
+// 3. Correct warning on out-of-line defaulted destructor (struct B)
+//
+// We expect exactly 1 warning (for struct B's out-of-line destructor).
+
+//--- header.h
+#pragma once
+
+// Negative cases: with modules, the destructor's redeclaration chain may
+// contain multiple entries, which can trigger false positives if not handled
+// correctly.
+
+// In-line defaulted destructor should NOT warn
+struct A {
+    ~A() = default;
+};
+
+// Implicit destructor should NOT warn
+struct X {
+    A a;
+};
+
+// Positive case: out-of-line defaulted destructor SHOULD warn
+struct B {
+    ~B();
+    int x;
+};
+
+//--- mymodule.cppm
+module;
+#include "header.h"
+export module mymodule;
+
+//--- main.cpp
+#include "header.h"
+import mymodule;
+
+// CHECK-MESSAGES: header.h:19:5: warning: class 'B' can be made trivially destructible
+B::~B() = default; // to-be-removed
+// CHECK-MESSAGES: :[[@LINE-1]]:4: note: destructor definition is here
+// CHECK-FIXES: // to-be-removed
+
+int main() {
+    X x;
+    B b;
+}

clang-tools-extra/test/lit.cfg.py

@@ -49,8 +49,8 @@
 # test_exec_root: The root path where tests should be run.
 config.test_exec_root = os.path.join(config.clang_tools_binary_dir, "test")
 
-# Tools need the same environment setup as clang (we don't need clang itself).
-llvm_config.clang_setup()
+# Set up clang for use in tests. Makes %clang substitution available.
+llvm_config.use_clang()
 
 if config.clang_tidy_staticanalyzer:
     config.available_features.add("static-analyzer")