dexonsmith accepted this revision. dexonsmith added a comment. This revision is now accepted and ready to land.
In D115374#3183776 <https://reviews.llvm.org/D115374#3183776>, @logan-5 wrote: > Removed `.flush()`es. Seems like this might be able to land without > https://reviews.llvm.org/D115421? Yup! LGTM if you remove the cleanups that are no longer related to this patch, and/or split them into separate follow-up NFC commits. Might be nice to split even the main commit into a few pieces (e.g., by functional area) to push separately, just in case something needs to be reverted due to a call to `SetBuffered()` in an unexpected place. Please make sure the commit message points at 65b13610a5226b84889b923bae884ba395ad084d <https://reviews.llvm.org/rG65b13610a5226b84889b923bae884ba395ad084d>, which made this correct. ================ Comment at: clang/include/clang/StaticAnalyzer/Checkers/SValExplainer.h:70 std::string Str; - llvm::raw_string_ostream OS(Str); - OS << "concrete memory address '" << I << "'"; - return OS.str(); + llvm::raw_string_ostream(Str) << "concrete memory address '" << I << "'"; + return Str; ---------------- Given there's no scope change needed, these cleanups are no longer related to the patch and are adding noise to the diff. I suggest committing them separately (or not at all). 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
