dexonsmith added a comment. In D115374#3181670 <https://reviews.llvm.org/D115374#3181670>, @logan-5 wrote:
> To be clear, it sounds like we should //either// add `.take()` for moving the > string out of `raw_string_ostream`'s string reference, //or// make > `raw_string_ostream` not need to be flushed (after which there won't be as > clear a use for `.take()`, since you can just use the underlying string > directly). > > I vote for the second option, making `raw_string_ostream` not need to be > flushed, since it allows for simpler code at the call site (`return Str;`), > permits NRVO at the call site, and avoids some possibly weird questions > `.take()` would bring with it (like whether it would ever be surprising that > it moves out of a //reference// that someone else might also have). > > If that's the direction that sounds best, I can submit an updated patch > sometime tomorrow. Yeah, probably one or the other. I'm leaning toward the no-flush approach as well, but I'd suggest making that a separate prep patch to reduce churn/dependencies in case there's some reason it needs to be reverted (e.g., compile time regression). Removing the no-longer-needed `.str()`s in a follow-up a few days later (rebasing this without the flushes) should be trivial. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D115374/new/ https://reviews.llvm.org/D115374 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
