Highlighter engine returns cached spans by reference; drop unneeded mutexes

HighlighterEngine::GetLine copied the span vector on every call (twice
on a miss) and did a dynamic_cast per call; it now returns the cached
entry (valid until the next call) and dispatches on Stateful().

PieceTable and HighlighterEngine took a mutex in their read paths while
their writes took none, and PieceTable.h claimed concurrent reads were
safe; they are not (reads update caches). Every Buffer is used from the
main thread only (the swap writer thread receives copies; the regex
large-stack thread runs while its caller waits), so the mutexes are
removed and the headers say so. ASan and the threaded tests under TSan
are clean.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015rcVQiiWyTzoNrsvLSaAfB

Buffer.h

@@ -44,7 +44,6 @@
 #include <cstdint>
 #include "syntax/HighlighterEngine.h"
 #include "Highlight.h"
-#include <mutex>
 
 // Edit mode determines which font class is used for a buffer.
 enum class EditMode { Code, Writing };

ImGuiRenderer.cc

@@ -438,7 +438,7 @@ ImGuiRenderer::Draw(Editor &ed)
 			}
 			// Draw syntax-colored runs (text above background highlights)
 			if (buf->SyntaxEnabled() && buf->Highlighter() && buf->Highlighter()->HasHighlighter()) {
-				kte::LineHighlight lh = buf->Highlighter()->GetLine(
+				const kte::LineHighlight &lh = buf->Highlighter()->GetLine(
 					*buf, static_cast<int>(i), buf->Version());
 				// Sanitize spans defensively: clamp to [0, line.size()], ensure end>=start, drop empties
 				struct SSpan {

PieceTable.cc

@@ -321,7 +321,6 @@ PieceTable::addPieceFront(Source src, std::size_t start, std::size_t len)
 void
 PieceTable::materialize() const
 {
-	std::lock_guard<std::mutex> lock(mutex_);
 	if (!dirty_) {
 		return;
 	}
@@ -401,7 +400,6 @@ PieceTable::coalesceNeighbors(std::size_t index)
 void
 PieceTable::InvalidateLineIndex() const
 {
-	std::lock_guard<std::mutex> lock(mutex_);
 	line_index_dirty_ = true;
 }
 
@@ -435,7 +433,6 @@ PieceTable::applyLineShift() const
 void
 PieceTable::lineIndexOnInsert(std::size_t offset, const char *text, std::size_t len) const
 {
-	std::lock_guard<std::mutex> lock(mutex_);
 	if (line_index_dirty_)
 		return;
 	// Line starts after the insertion point move right; a start exactly at
@@ -461,7 +458,6 @@ PieceTable::lineIndexOnInsert(std::size_t offset, const char *text, std::size_t
 void
 PieceTable::lineIndexOnDelete(std::size_t offset, std::size_t len) const
 {
-	std::lock_guard<std::mutex> lock(mutex_);
 	if (line_index_dirty_)
 		return;
 	// Starts in (offset, offset+len] followed a deleted newline: drop them.
@@ -482,7 +478,6 @@ PieceTable::lineIndexOnDelete(std::size_t offset, std::size_t len) const
 void
 PieceTable::RebuildLineIndex() const
 {
-	std::lock_guard<std::mutex> lock(mutex_);
 
 	if (!line_index_dirty_) {
 		return;
@@ -930,7 +925,6 @@ PieceTable::GetRange(std::size_t byte_offset, std::size_t len) const
 
 	// Fast path: return cached value if version/offset/len match
 	{
-		std::lock_guard<std::mutex> lock(mutex_);
 		if (range_cache_.valid && range_cache_.version == version_ &&
 		    range_cache_.off == byte_offset && range_cache_.len == len) {
 			return range_cache_.data;
@@ -940,7 +934,6 @@ PieceTable::GetRange(std::size_t byte_offset, std::size_t len) const
 	std::string out;
 	out.reserve(len);
 	if (!dirty_) {
-		std::lock_guard<std::mutex> lock(mutex_);
 		// Already materialized; slice directly
 		out.assign(materialized_.data() + static_cast<std::ptrdiff_t>(byte_offset), len);
 	} else {
@@ -965,7 +958,6 @@ PieceTable::GetRange(std::size_t byte_offset, std::size_t len) const
 
 	// Update cache
 	{
-		std::lock_guard<std::mutex> lock(mutex_);
 		range_cache_.valid   = true;
 		range_cache_.version = version_;
 		range_cache_.off     = byte_offset;
@@ -984,7 +976,6 @@ PieceTable::Find(const std::string &needle, std::size_t start) const
 	if (start > total_size_)
 		return std::numeric_limits<std::size_t>::max();
 	{
-		std::lock_guard<std::mutex> lock(mutex_);
 		if (find_cache_.valid &&
 		    find_cache_.version == version_ &&
 		    find_cache_.needle == needle &&
@@ -996,7 +987,6 @@ PieceTable::Find(const std::string &needle, std::size_t start) const
 	materialize();
 	std::size_t pos;
 	{
-		std::lock_guard<std::mutex> lock(mutex_);
 		pos = materialized_.find(needle, start);
 		if (pos == std::string::npos)
 			pos = std::numeric_limits<std::size_t>::max();

PieceTable.h

@@ -33,7 +33,8 @@
  *
  * Implementation notes:
  * - Consolidation heuristics prevent piece fragmentation (configurable via SetConsolidationParams)
- * - Thread-safe for concurrent reads (mutex protects caches and lazy rebuilds)
+ * - Not thread-safe: even reads update caches. Buffers are used from the
+ *   main thread only (the swap writer thread gets copies of the bytes).
  * - Version tracking invalidates caches on mutations
  */
 #pragma once
@@ -43,7 +44,6 @@
 #include <ostream>
 #include <vector>
 #include <limits>
-#include <mutex>
 #include <string_view>
 
 #include "TextSpan.h"
@@ -297,6 +297,4 @@ private:
 
 	mutable RangeCache range_cache_;
 	mutable FindCache find_cache_;
-
-	mutable std::mutex mutex_;
 };

QtFrontend.cc

@@ -308,7 +308,7 @@ protected:
 					// as operators).
 					if (buf->SyntaxEnabled() && buf->Highlighter() && buf->Highlighter()->
 					    HasHighlighter()) {
-						kte::LineHighlight lh = buf->Highlighter()->GetLine(
+						const kte::LineHighlight &lh = buf->Highlighter()->GetLine(
 							*buf, static_cast<int>(i), buf->Version());
 						struct SSpan {
 							std::size_t s;

TerminalRenderer.cc

@@ -163,7 +163,7 @@ TerminalRenderer::Draw(Editor &ed)
 				std::vector<kte::HighlightSpan> sane_spans;
 				if (buf->SyntaxEnabled() && buf->Highlighter() && buf->Highlighter()->
 				    HasHighlighter()) {
-					kte::LineHighlight lh_val = buf->Highlighter()->GetLine(
+					const kte::LineHighlight &lh_val = buf->Highlighter()->GetLine(
 						*buf, static_cast<int>(li), buf->Version());
 					// Sanitize defensively: clamp to [0, line.size()], ensure end>=start, drop empties
 					const std::size_t line_len = line.size();

syntax/HighlighterEngine.cc

@@ -14,7 +14,6 @@ HighlighterEngine::~HighlighterEngine() = default;
 void
 HighlighterEngine::SetHighlighter(std::unique_ptr<LanguageHighlighter> hl)
 {
-	std::lock_guard<std::mutex> lock(mtx_);
 	hl_ = std::move(hl);
 	clear_caches_locked();
 }
@@ -39,38 +38,26 @@ HighlighterEngine::invalidate_from_locked(int row) const
 }
 
 
-LineHighlight
+const LineHighlight &
 HighlighterEngine::GetLine(const Buffer &buf, int row, std::uint64_t buf_version) const
 {
-	std::lock_guard<std::mutex> lock(mtx_);
 	if (!have_version_ || buf_version != version_) {
 		// Content changed without OnEdit: nothing cached can be trusted.
 		clear_caches_locked();
 		version_      = buf_version;
 		have_version_ = true;
 	}
-	auto it = cache_.find(row);
-	if (it != cache_.end())
-		return it->second; // return by value (copy)
+	if (auto it = cache_.find(row); it != cache_.end())
+		return it->second;
 
 	LineHighlight result;
 	result.version = buf_version;
-
-	if (!hl_ || row < 0) {
-		cache_[row] = result;
-		return result;
-	}
-
-	auto *stateful = dynamic_cast<StatefulHighlighter *>(hl_.get());
-	if (!stateful) {
+	if (hl_ && row >= 0 && !hl_->Stateful()) {
 		hl_->HighlightLine(buf, row, result.spans);
-		cache_[row] = result;
-		trim_cache_locked(row);
-		return result;
-	}
-
-	// Stateful: continue from the last row with a known end state (or the
-	// row just above the target, if that is known), caching every row's
+	} else if (hl_ && row >= 0) {
+		const auto *stateful = static_cast<const StatefulHighlighter *>(hl_.get());
+		// Continue from the last row with a known end state (or the row
+		// just above the target, if that is known), caching every row's
 		// spans and state along the way so neighbouring rows are cache hits.
 		const int known = static_cast<int>(states_.size()) - 1;
 		const int start = std::min(known, row - 1);
@@ -86,16 +73,17 @@ HighlighterEngine::GetLine(const Buffer &buf, int row, std::uint64_t buf_version
 			else
 				states_[static_cast<std::size_t>(r)] = state;
 			if (r == row)
-			result = lh;
+				result = std::move(lh);
 			else if (row - r <= kCacheNear)
 				cache_.insert_or_assign(r, std::move(lh));
-		// Rows further above only needed their end state (kept in states_):
-		// caching their spans too held about 10x the file size in memory
-		// after jumping to the end of a large file.
+			// Rows further above only needed their end state (kept in
+			// states_): caching their spans too held about 10x the file size
+			// in memory after jumping to the end of a large file.
 		}
-	cache_[row] = result;
+	}
+	// Trim first: the entry returned must survive until the next call.
 	trim_cache_locked(row);
-	return result;
+	return cache_.insert_or_assign(row, std::move(result)).first->second;
 }
 
 
@@ -112,7 +100,6 @@ HighlighterEngine::trim_cache_locked(const int row) const
 void
 HighlighterEngine::InvalidateFrom(int row)
 {
-	std::lock_guard<std::mutex> lock(mtx_);
 	invalidate_from_locked(row);
 }
 
@@ -120,7 +107,6 @@ HighlighterEngine::InvalidateFrom(int row)
 void
 HighlighterEngine::OnEdit(int row, std::uint64_t buf_version)
 {
-	std::lock_guard<std::mutex> lock(mtx_);
 	if (have_version_)
 		invalidate_from_locked(row);
 	version_      = buf_version;

syntax/HighlighterEngine.h

@@ -1,11 +1,11 @@
-// HighlighterEngine.h - caching layer for per-line highlights
+// HighlighterEngine.h - caching layer for per-line highlights (main thread
+// only, like the Buffer it reads)
 #pragma once
 
 #include <cstdint>
 #include <map>
 #include <memory>
 #include <vector>
-#include <mutex>
 
 #include "../Highlight.h"
 #include "LanguageHighlighter.h"
@@ -21,10 +21,10 @@ public:
 
 	void SetHighlighter(std::unique_ptr<LanguageHighlighter> hl);
 
-	// Retrieve highlights for a given line and buffer version.
-	// Returns a copy to avoid lifetime issues across threads/renderers.
-	// If cache is stale, recompute using the current highlighter.
-	LineHighlight GetLine(const Buffer &buf, int row, std::uint64_t buf_version) const;
+	// Highlights for a line at a buffer version, computed if not cached.
+	// The reference is valid until the next call on this engine (a later
+	// lookup may evict the entry); copy it to keep it longer.
+	const LineHighlight &GetLine(const Buffer &buf, int row, std::uint64_t buf_version) const;
 
 	// Invalidate cached lines from row (inclusive)
 	void InvalidateFrom(int row);
@@ -74,8 +74,5 @@ private:
 	static constexpr std::size_t kCacheMax = 16384;
 
 	void trim_cache_locked(int row) const;
-
-	// Guards the caches above.
-	mutable std::mutex mtx_;
 };
 } // namespace kte