dblaikie added inline comments.
================ Comment at: clang/lib/Serialization/ASTReader.cpp:1457 + const llvm::compression::Format F = + Blob.size() >= 2 && memcmp(Blob.data(), "\x1f\x8b", 2) == 0 + ? llvm::compression::Format::Zlib ---------------- MaskRay wrote: > dblaikie wrote: > > tschuett wrote: > > > Could you put the magic number into a named constant? > > & perhaps we should have/test a magic number for zstd too, so it's clear > > it's one or the other and not something else added in the future (since > > this isn't reving the bitcode version or anything - reading zstd compressed > > data with the library before this version will I guess/hopefully appear > > corrupted, but we could avoid that being the failure mode in the future > > when another compression scheme is added by checking explicitly for > > zstd/zlib now and reporting unknown compression scheme otherwise?)? > > Could you put the magic number into a named constant? > > I think a constant doesn't improve readability of this used-once value with a > comment. > > > & perhaps we should have/test a magic number for zstd too, so it's clear > > it's one or the other and not something else added in the future > > Since the version number keeps increasing, when a new algorithm is added, > they will see a version mismatch error... > > & perhaps we should have/test a magic number for zstd too, so it's clear > > it's one or the other and not something else added in the future > > Since the version number keeps increasing, when a new algorithm is added, > they will see a version mismatch error... Oh, right, sorry, got muddled - thinking bitcode & then thinking LLVM IR bitcode guarantees, super stable, but the AST's super unstable/closely version locked anyway. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137885/new/ https://reviews.llvm.org/D137885 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
