[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