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: > > [...] > > 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. > > Resetting the allocator is a pretty expensive operation, so we choose not to > (and prefer to just 'leak' or continue allocation) unless we opt-in. This is > one of those things that has significant compile-time implications.
Ah I see. Well, it might deserve a comment then. https://github.com/llvm/llvm-project/pull/191697 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
