[PATCH] D130516: [Support] compression classes

[Cole Kissane via Phabricator via cfe-commits]

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:
> 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.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D130516/new/

https://reviews.llvm.org/D130516