ckissane 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(); + ---------------- dblaikie wrote: > 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? good catch 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
