jhenderson added a comment. Overall, this seems reasonable to me. I have a slight concern that it might cause surprising failures for downstream users, that aren't picked up at build time (due to implicit conversions), but I don't think you need to worry too much about that.
================ Comment at: llvm/include/llvm/Support/MemoryBuffer.h:78-80 /// if successful, otherwise returning null. If FileSize is specified, this /// means that the client knows that the file exists and that it has the /// specified size. ---------------- You need to remove the reference to FileSize from the description here. ================ Comment at: llvm/lib/Support/MemoryBuffer.cpp:108 static ErrorOr<std::unique_ptr<MB>> getFileAux(const Twine &Filename, int64_t FileSize, uint64_t MapSize, + uint64_t Offset, bool IsText, bool RequiresNullTerminator, ---------------- Are there users of the `FileSize` here? If not, can you repeat the change for this function? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D99182/new/ https://reviews.llvm.org/D99182 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
