[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