[PATCH] D124635: Frontend: Delete output streams before closing CompilerInstance outputs
Ben Langmuir via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Thu Apr 28 16:23:50 PDT 2022
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
More information about the cfe-commits
mailing list