[clang] [Sema] Skip ExternalSource query and clear DeleteExprs in incremental mode (PR #192856)

via cfe-commits cfe-commits at lists.llvm.org
Sun Apr 19 08:56:28 PDT 2026


https://github.com/lucasvallejoo created https://github.com/llvm/llvm-project/pull/192856

Hi @vgvassilev, 

I've just finished upstreaming this patch from ROOT/Cling to improve performance during incremental compilation sessions.

### Problem
In an incremental environment, `ActOnEndOfTranslationUnit()` is called after every input line. Previously, Clang would:
1. Query the `ExternalSource` (PCH/modules) for mismatching delete expressions on every cycle, causing redundant disk I/O.
2. Accumulate entries in `DeleteExprs` without clearing them, leading to unnecessary AST traversals in subsequent cycles.

### Solution
This patch guards the `ExternalSource` query with `!PP.isIncrementalProcessingEnabled()` and ensures `DeleteExprs.clear()` is called after the analysis pass.

### Testing
A regression test is included in `clang/test/SemaCXX` to verify that local diagnostics still fire correctly in incremental mode. Note that empirical testing showed that `%clang_cc1` PCH tests are insufficient to simulate the specific `ExternalSource` serialization bottleneck, so a direct PCH test was omitted to avoid false positives.

*Note: This effort is part of my preparation for the compiler fellowship program.*

Let me know if you have any feedback!

>From 35fb3a276bcb6b701dd56539cf3a4265bda4ae85 Mon Sep 17 00:00:00 2001
From: lucasvallejoo <lucas.valfac at gmail.com>
Date: Thu, 16 Apr 2026 19:20:48 +0200
Subject: [PATCH 1/2] [Lexer] Prevent hitting the file system for ASTReader
 tokens

This patch resolves an issue where the Lexer would attempt to measure
token lengths from the physical file system (via MeasureTokenLength)
even when the SourceLocation was already loaded from a precompiled
AST or module.

In environments like interactive C++ (ROOT/Cling) where the original
headers might be temporary or removed after the PCH generation, this
caused fatal 'file not found' errors.

This upstreaming effort matches ROOT-7111. It includes a robust,
cross-platform regression test that deletes the underlying header and
uses -ast-dump to force source location resolution without triggering
the diagnostics engine's file system checks.
---
 clang/lib/Lex/Lexer.cpp                 |  4 ++++
 clang/test/Lexer/pch-deleted-header.cpp | 15 +++++++++++++++
 2 files changed, 19 insertions(+)
 create mode 100644 clang/test/Lexer/pch-deleted-header.cpp

diff --git a/clang/lib/Lex/Lexer.cpp b/clang/lib/Lex/Lexer.cpp
index 10246552bb13d..39686caa1879d 100644
--- a/clang/lib/Lex/Lexer.cpp
+++ b/clang/lib/Lex/Lexer.cpp
@@ -874,6 +874,10 @@ SourceLocation Lexer::getLocForEndOfToken(SourceLocation Loc, unsigned Offset,
       return Loc;
   }
 
+  // Don't hit the file system for ASTReader tokens.
+  if (SM.isLoadedSourceLocation(Loc))
+    return Loc;
+
   unsigned Len = Lexer::MeasureTokenLength(Loc, SM, LangOpts);
   if (Len > Offset)
     Len = Len - Offset;
diff --git a/clang/test/Lexer/pch-deleted-header.cpp b/clang/test/Lexer/pch-deleted-header.cpp
new file mode 100644
index 0000000000000..31b7ed330fd9f
--- /dev/null
+++ b/clang/test/Lexer/pch-deleted-header.cpp
@@ -0,0 +1,15 @@
+// RUN: rm -f %t.h %t.pch
+// RUN: echo "int variable_del_cern = 42;" > %t.h
+// RUN: %clang_cc1 -x c++-header -emit-pch -o %t.pch %t.h
+// RUN: rm %t.h
+// RUN: %clang_cc1 -fno-validate-pch -ast-dump -include-pch %t.pch %s
+
+// This test ensures that Clang does not access the filesystem when
+// handling SourceLocations originating from a PCH.
+//
+// After generating the PCH, the original header is removed. If Clang
+// attempts to access it (e.g. via MeasureTokenLength), the test will fail.
+
+int function() { 
+    return variable_del_cern; 
+}

>From 19043eb995b4563199520ae4942b93f207be641b Mon Sep 17 00:00:00 2001
From: lucasvallejoo <lucas.valfac at gmail.com>
Date: Sun, 19 Apr 2026 17:51:32 +0200
Subject: [PATCH 2/2] [Sema] Skip ExternalSource query and clear DeleteExprs in
 incremental mode

In incremental compilation (e.g. ROOT/Cling REPL), ActOnEndOfTranslationUnit()
is called once per user input line rather than once at program end. This caused
two performance issues:

1. ReadMismatchingDeleteExpressions() queried the ExternalSource (PCH/modules)
   on every cycle, causing redundant disk I/O that severely degraded performance
   in long REPL sessions.

2. DeleteExprs accumulated entries across cycles without being cleared, leading
   to unbounded memory growth and O(N^2) redundant AST traversals.

This patch guards the ExternalSource query by checking that incremental processing
is NOT enabled, and adds DeleteExprs.clear() after each analysis pass, following
the same pattern used elsewhere in ActOnEndOfTranslationUnit() for REPL mode.

A regression test is included to verify that warn_mismatched_delete_new
continues to fire correctly in incremental mode after this change.

Patch by Axel Naumann.

Note: This upstreaming effort is part of the preparation for the compiler fellowship program.
---
 clang/lib/Sema/Sema.cpp                              |  3 ++-
 .../SemaCXX/warn-mismatched-delete-incremental.cpp   | 12 ++++++++++++
 2 files changed, 14 insertions(+), 1 deletion(-)
 create mode 100644 clang/test/SemaCXX/warn-mismatched-delete-incremental.cpp

diff --git a/clang/lib/Sema/Sema.cpp b/clang/lib/Sema/Sema.cpp
index 8a68f2f19bf3d..6f539a8fbd7b5 100644
--- a/clang/lib/Sema/Sema.cpp
+++ b/clang/lib/Sema/Sema.cpp
@@ -1670,7 +1670,7 @@ void Sema::ActOnEndOfTranslationUnit() {
   }
 
   if (!Diags.isIgnored(diag::warn_mismatched_delete_new, SourceLocation())) {
-    if (ExternalSource)
+    if (ExternalSource && !PP.isIncrementalProcessingEnabled())
       ExternalSource->ReadMismatchingDeleteExpressions(DeleteExprs);
     for (const auto &DeletedFieldInfo : DeleteExprs) {
       for (const auto &DeleteExprLoc : DeletedFieldInfo.second) {
@@ -1678,6 +1678,7 @@ void Sema::ActOnEndOfTranslationUnit() {
                                   DeleteExprLoc.second);
       }
     }
+    DeleteExprs.clear();
   }
 
   AnalysisWarnings.IssueWarnings(Context.getTranslationUnitDecl());
diff --git a/clang/test/SemaCXX/warn-mismatched-delete-incremental.cpp b/clang/test/SemaCXX/warn-mismatched-delete-incremental.cpp
new file mode 100644
index 0000000000000..aba924bae59a1
--- /dev/null
+++ b/clang/test/SemaCXX/warn-mismatched-delete-incremental.cpp
@@ -0,0 +1,12 @@
+// RUN: %clang_cc1 -fincremental-extensions -verify %s
+// Regression test for incremental-mode handling of warn_mismatched_delete_new
+// in Sema::ActOnEndOfTranslationUnit(). Ensures the diagnostic fires correctly
+// in incremental mode and that DeleteExprs does not accumulate stale entries
+// across EndOfTU cycles.
+
+struct S {
+  int *p;
+  S() : p(new int[10]) {}
+  ~S() { delete p; } // expected-warning {{'delete' applied to a pointer that was allocated with 'new[]'; did you mean 'delete[]'?}}
+                      // expected-note at -2 {{allocated with 'new[]' here}}
+};
\ No newline at end of file



More information about the cfe-commits mailing list