[clang] [clang][timers][modules] Fix a timer being started when it's running (PR #154231)

Alan Zhao via cfe-commits cfe-commits at lists.llvm.org
Tue Aug 19 14:07:41 PDT 2025


================
@@ -11003,8 +11003,9 @@ void ASTReader::diagnoseOdrViolations() {
 }
 
 void ASTReader::StartedDeserializing() {
-  if (++NumCurrentElementsDeserializing == 1 && ReadTimer.get())
-    ReadTimer->startTimer();
+  if (llvm::Timer *T = ReadTimer.get();
----------------
alanzhao1 wrote:

`TimeRegion` is pretty nifty - TIL, thanks!

I'm not sure that RAII improves readability here. RAII is really useful when an object is created and destroyed within the same stack frame, e.g.

```cpp
Timer *T = // ...
{
  TimeRegion TR(T); // Timer starts

  // some code

} // TR is automatically destroyed, no timer bugs because we forgot to call stopTimer()
```  

But in this case, we're explicitly calling `TimeRegion`'s destructor [0], so it's not less wordier than just caling `Timer::startTimer()` (and `stopTimer()`).

Moreover, `TimeRegion` [doesn't check whether or not the timer has started yet](https://github.com/llvm/llvm-project/blob/6127e46ff86bc660c0de5e7ece764005c91a1aaa/llvm/include/llvm/Support/Timer.h#L158-L160), so we still need to check for `!T->isRunning()`.

[0]: More accurately, we're explicitly making an implicit call to the destructor via `std::optional::reset()`.

https://github.com/llvm/llvm-project/pull/154231


More information about the cfe-commits mailing list