MaskRay added inline comments.
================ Comment at: llvm/docs/ReleaseNotes.rst:183-191 +* Refactor compression namespaces across the project, making way for a possible + introduction of alternatives to zlib compression in the llvm toolchain. + Changes are as follows: + * Remove crc32 from zlib compression namespace, people should use the ``llvm::crc32`` instead. + * Relocate the ``llvm::zlib`` namespace to ``llvm::compression::zlib``. + * Code that explictly needs ``zlib`` compression (IE zlib elf debug sections) should use ``llvm::compression::zlib``. + * Code interfacing with compressed profile data should use ``llvm::compression::profile``. ---------------- leonardchan wrote: > I think maybe even more of these could be split into further patches. I would > expect that this patch contain just the namespace refactoring but not > necessarily any code deletion or cmake changes. I think this could be: > > - One patch that's just the namespace changes; anything else like variable > name changes or string changes would be in separate patches, then this patch > would be pure RFC and can get a quick LGTM > - One followup patch to this that adds `AlgorithmName` to the nested `zlib` > namespace. > - One patch that removes crc32 (and its test) > - One patch for cmake changes > - One patch for the `zlib_unavailable -> compression_unavailable` change and > logging string changes in profdata > > And if necessary, each of them would have an appropriate ReleaseNodes.rst > update. I agree with this plan. Changes like `zlib_unavailable` need to bring attention to the usual contributors/reviewers in the area. ================ Comment at: llvm/lib/Support/Compression.cpp:26 -#if LLVM_ENABLE_ZLIB +using namespace compression; + ---------------- ================ Comment at: llvm/unittests/Support/CompressionTest.cpp:22 +using namespace compression; + ---------------- Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128754/new/ https://reviews.llvm.org/D128754 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
