[PATCH] D115421: [Support] No longer require .flush()ing raw_string_ostream

Duncan P. N. Exon Smith via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Dec 9 18:43:40 PST 2021


dexonsmith added a comment.

In D115421#3184249 <https://reviews.llvm.org/D115421#3184249>, @logan-5 wrote:

> To be quite honest, in regards to the big `.str()` refactor (whether changing its type or deleting it), I feel I may have started to bite off more than I can chew here. There are at least a few thousand call sites of `.str()` throughout LLVM and its subprojects,

(FWIW, I bet most of those are unrelated to `raw_string_ostream`. Removing the function would probably find you a much smaller list.)

> and as just a very occasional contributor who will be a distant stranger to most of them, I don't feel equipped or qualified to tackle auditing and fixing them myself. (Not to mention that it's orders of magnitude more work than I had planned to contribute with my initial patch.) I just wanted to optimize a few `return`s, and now we're talking about making a huge number of changes across the entire repo. I'm happy to bring this current patch through to landing, but when it comes to the big `.str()` purge I might have to humbly step back.

Fair enough! In that case, I suggest just removing the `flush()` from `str()` (as you have here) and adding a TODO to consider deleting it or making it return StringRef. Then at least anyone who looks can see it's obviously uninteresting; and you and/or others can incrementally delete the uninteresting calls over time.

Note also: git-grep gives me only 10 hits across llvm-project `SetBuffered()`; the only two outside of `raw_ostream` and its tests are `llvm::errs().SetBuffered()` in clangd. In case you're still worried about whether it's safe to do.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D115421/new/

https://reviews.llvm.org/D115421



More information about the llvm-commits mailing list