benlangmuir added a comment. LGTM, although I made a suggestion about the spelling.
> This fixes theoretical bugs related to proxy streams, which may have > cleanups to run in their destructor. E.g., buffer_ostream supports > pwrite() by holding all content in a buffer until destruction time. The general point about streams with destructors is well taken, but it seems a bit broken that `buffer_ostream` cannot `flush`. I guess the layering it just weird in that we have buffering in the base class, but `buffer_ostream` wants to do something different. Could we afford to make `flush` virtual? Not something needed for this patch. ================ Comment at: clang/lib/ExtractAPI/ExtractAPIConsumer.cpp:796 SGSerializer.serialize(*OS); - OS->flush(); + OS = nullptr; } ---------------- Nit: `OS.reset(nullptr);` makes it more obvious this is a `unique_ptr` and not a raw pointer. Same for the other cases as well. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124635/new/ https://reviews.llvm.org/D124635 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
