[Mlir-commits] [mlir] [mlir][bytecode] Fix SourceMgr lifetime extension in BytecodeReader (PR #185321)

[Jacques Pienaar]

https://github.com/jpienaar commented:

IMHO the usage of this was not to expand it while the BytecodeReader is active, but specific to resource blob's backing data. So its for where one has a parsed resource, keeping the backing alive. That makes sense to me as its about avoiding a copy. While I don't see a reason to have BytecodeReader persist beyond SourceMgr, not saying there are none but I just don't know them off hand. I'd expect almost always a populate to be followed by a destroy, as the populated operation* is the artifact now. So this seems WAI as its only about extending lifetime in that narrow case. 

I could be wrong, and we check with @River707 if he recalls.

If there is a reason to extend beyond that, then we could look at that (e.g,. perhaps you've found cost of creating a reader expensive, then we could look at the performance profile). So independent of historical precedent, what is the pattern where this is needed and the other is worse fit?

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