akhuang added inline comments.
================ Comment at: clang/lib/Frontend/CompilerInstance.cpp:857-858 + OS.reset(new llvm::raw_fd_ostream(Temp->FD, /*shouldClose=*/false, + Binary ? llvm::sys::fs::OF_None + : llvm::sys::fs::OF_Text)); + OSFile = std::string(TempPath.str()); ---------------- Quuxplusone wrote: > amccarth wrote: > > rnk wrote: > > > I think the bug is here: the third parameter is `bool unbuffered`, not > > > file flags, so we are opening the file for unbuffered writing, and that > > > is really slow. > > Yowza! In addition to a fix, we need some memes. > > > > * Why do we even have that lever? > > * This constructor has too many overloads. Please remove three. > > * [Facepalm] `bool` parameters. [Double facepalm] Negative `bool` > > parameter names that default to `false`. > > In addition to a fix, we need some memes. > > https://quuxplusone.github.io/blog/2020/04/18/default-function-arguments-are-the-devil/ > ;) > I count 8 different signatures for `raw_fd_ostream`'s constructor, 3 of which > come from the offending default-argument'ed overload. And none of them are > `explicit`, which means e.g. `{42, false}` is a valid argument for a function > taking `const raw_fd_ostream&`. > > Unrelated to argument lists, but I would add "using raw `new` instead of > `std::make_unique`" to this line's lengthy list of sins. oh whoops, thanks for the catch - for now I guess I'll just remove the extra parameter Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D102736/new/ https://reviews.llvm.org/D102736 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
