vgvassilev wrote: > > My concerns here were memory-pressure related, and I think I've been > > convinced it is not in practice sufficient concern. @AaronBallman : I think > > this is superior in every way from #226612 other than in 'amount of text > > changed', and the stepping-stone isn't worth it. > > You good with this one? > > "Good" might be a stretch -- I can probably hold my nose. My concern with > either is that we're going from a single source of truth as to what the AST > context is to "each `DeclContext` can have its own, at least in theory", so > now we may start seeing people try to assert invariants that shouldn't need > to be there if we continue to have only the TU decl holding the `ASTContext`. > So I think caching the TU decl is actually cleaner than caching the AST > context in some ways. (We do allow multiple AST contexts in one situation > that I'm aware of: when loading modules, before having merged everything > together, but I'm basing my concerns on the understanding that once the AST > is fully loaded, there should only ever be _one_ `ASTContext` for a given > tree. If that understanding is wrong, that may change things. CC @Bigcheese > @vgvassilev @ChuanqiXu9)
IIUC the intent of this PR is to speed up the operation `Decl::getASTContext`. We used to consider that having to go via the `Decl::getASTContext` was a bug in layering and that interface was a "temporary workaround" that was supposed to be deleted. Does MLIR have some idiom that differs from that (maybe outdated) understanding? https://github.com/llvm/llvm-project/pull/223047 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
