dblaikie added inline comments.
================ Comment at: clang/lib/Serialization/ASTWriter.cpp:2003-2004 // consumers will not want its contents. + llvm::compression::CompressionAlgorithm CompressionScheme = + llvm::compression::ZlibCompressionAlgorithm(); + ---------------- ckissane wrote: > dblaikie wrote: > > Doesn't this cause slicing & end up with the base implementation? > > > > (also the base class `CompressionAlgorithm` has no virtual functions, so > > I'm not sure how this is meant to work - does this code all work? Then I > > must be missing some things - how does this work?) > You are correct to observe that this patch does not fully pass around > pointers to instances of the classes, however, because I don't pass pointers > and the currently repetitive nature of the compression classes, this still > functions correctly. > In short, a follow-up patch (which I will shortly upload) will convert this > to using class instances and passing those around. > Including reworking functions throughout llvm-project to take advantage of > this. > I am aiming to take this 2 step process to cut down on making an already > large pass larger. > Let me know if you have any concerns or ideas. But I'm not sure how this patch works correctly - wouldn't the line below (`CompressionScheme.supported()`) call `CompressionAlgorithm::supported()` which always returns false? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D130516/new/ https://reviews.llvm.org/D130516 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
