steakhal wrote:
I'm glad to see alignment here. Thank you.
I didn't expect such a great interest. In this case, I'd raise another point
(but without expanding the current PR):
In the `CodeGenAction.cpp` we have this code:
```c++
if (CodeGenOpts.ClearASTBeforeBackend) {
LLVM_DEBUG(llvm::dbgs() << "Clearing AST...\n");
// Access to the AST is no longer available after this.
// Other things that the ASTContext manages are still available, e.g.
// the SourceManager. It'd be nice if we could separate out all the
// things in ASTContext used after this point and null out the
// ASTContext, but too many various parts of the ASTContext are still
// used in various parts.
C.cleanup();
C.getAllocator().Reset(); // <--- here
}
```
And this raises me a question, why is `C.getAllocator().Reset();` not done part
of `cleanup` already?
IMO it's very suspicious.
What I suspected was that it is used from `ASTContext::PrintStats` thus
reseting it would have an effect on debuggability.
However, this `ASTContext::PrintStats` is called from `clang::ParseAST` after
`Consumer->HandleTranslationUnit(S.getASTContext());` is called. This means
that even today, this allocator is conditionally reset.
So I really have no idea why it's not in `cleanup`; but I think I'll leave it
for the experts to judge.
https://github.com/llvm/llvm-project/pull/191697
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
